2023-04-15 10:42:15

by Adam Ford

Subject: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

If there is more than one lane, the HFP, HBP, and HSA is calculated in
bytes/pixel, then they are divided amongst the different lanes with some
additional overhead. This is necessary to achieve higher resolutions while
keeping the pixel clocks lower as the number of lanes increase.

Signed-off-by: Adam Ford <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index e0a402a85787..1ccbad4ea577 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -215,6 +215,7 @@
#define DSI_RX_FIFO_SIZE 256
#define DSI_XFER_TIMEOUT_MS 100
#define DSI_RX_FIFO_EMPTY 0x30800002
+#define DSI_HSYNC_PKT_OVERHEAD 6

#define OLD_SCLK_MIPI_CLK_NAME "pll_clk"

@@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
| DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);

- reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
- | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
- samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
+ /*
+ * If there is more than one lane, the HFP, HBP, and HSA
+ * is calculated in bytes/pixel, then they are divided
+ * amongst the different lanes with some additional
+ * overhead correction
+ */
+ if (dsi->lanes > 1) {
+ u32 hfp, hbp, hsa;
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format) / 8;
+
+ hfp = ((m->hsync_start - m->hdisplay) * bpp) / dsi->lanes;
+ hfp -= (hfp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
+
+ hbp = ((m->htotal - m->hsync_end) * bpp) / dsi->lanes;
+ hbp -= (hbp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;

- reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
- | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
- samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
+ hsa = ((m->hsync_end - m->hsync_start) * bpp) / dsi->lanes;
+ hsa -= (hsa > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
+
+ reg = DSIM_MAIN_HFP(hfp) | DSIM_MAIN_HBP(hbp);
+ samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
+
+ reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
+ | DSIM_MAIN_HSA(hsa);
+ samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
+ } else {
+ reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
+ | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
+ samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
+
+ reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
+ | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
+ samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
+ }
}
reg = DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
DSIM_MAIN_VRESOL(m->vdisplay, num_bits_resol);
--
2.39.2

2023-04-15 10:42:16

by Adam Ford

Subject: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
and max values for M and the frequency range for the VCO_out
calculator were incorrect. This also appears to be the case for the
imx8mn and imx8mp.

To fix this, make new variables to hold the min and max values of m
and the minimum value of VCO_out, and update the PMS calculator to
use these new variables instead of using hard-coded values to keep
the backwards compatibility with other parts using this driver.

Fixes: 4d562c70c4dc ("drm: bridge: samsung-dsim: Add i.MX8M Mini/Nano support")
Signed-off-by: Adam Ford <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 22 ++++++++++++++++++++--
include/drm/bridge/samsung-dsim.h | 3 +++
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 1ccbad4ea577..9fec32b44e05 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -406,6 +406,9 @@ static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
.num_bits_resol = 11,
.pll_p_offset = 13,
.reg_values = reg_values,
+ .m_min = 41,
+ .m_max = 125,
+ .vco_min = 500,
};

static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
@@ -419,6 +422,9 @@ static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
.num_bits_resol = 11,
.pll_p_offset = 13,
.reg_values = reg_values,
+ .m_min = 41,
+ .m_max = 125,
+ .vco_min = 500,
};

static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
@@ -430,6 +436,9 @@ static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
.num_bits_resol = 11,
.pll_p_offset = 13,
.reg_values = reg_values,
+ .m_min = 41,
+ .m_max = 125,
+ .vco_min = 500,
};

static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
@@ -442,6 +451,9 @@ static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
.num_bits_resol = 12,
.pll_p_offset = 13,
.reg_values = exynos5433_reg_values,
+ .m_min = 41,
+ .m_max = 125,
+ .vco_min = 500,
};

static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
@@ -454,6 +466,9 @@ static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
.num_bits_resol = 12,
.pll_p_offset = 13,
.reg_values = exynos5422_reg_values,
+ .m_min = 41,
+ .m_max = 125,
+ .vco_min = 500,
};

static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
@@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
*/
.pll_p_offset = 14,
.reg_values = imx8mm_dsim_reg_values,
+ .m_min = 64,
+ .m_max = 1023,
+ .vco_min = 1050,
};

static const struct samsung_dsim_driver_data *
@@ -548,12 +566,12 @@ static unsigned long samsung_dsim_pll_find_pms(struct samsung_dsim *dsi,
tmp = (u64)fout * (_p << _s);
do_div(tmp, fin);
_m = tmp;
- if (_m < 41 || _m > 125)
+ if (_m < driver_data->m_min || _m > driver_data->m_max)
continue;

tmp = (u64)_m * fin;
do_div(tmp, _p);
- if (tmp < 500 * MHZ ||
+ if (tmp < driver_data->vco_min * MHZ ||
tmp > driver_data->max_freq * MHZ)
continue;

diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index ba5484de2b30..a088d84579bc 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -59,6 +59,9 @@ struct samsung_dsim_driver_data {
unsigned int num_bits_resol;
unsigned int pll_p_offset;
const unsigned int *reg_values;
+ u16 m_min;
+ u16 m_max;
+ u64 vco_min;
};

struct samsung_dsim_host_ops {
--
2.39.2

2023-04-15 10:42:18

by Adam Ford

Subject: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
having an entry in the device tree for samsung,pll-clock-frequency.

Signed-off-by: Adam Ford <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 9fec32b44e05..73f0c3fbbdf5 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
struct device_node *node = dev->of_node;
int ret;

- ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
- &dsi->pll_clk_rate);
- if (ret < 0)
- return ret;
-
ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
&dsi->burst_clk_rate);
if (ret < 0)
@@ -1823,13 +1818,18 @@ int samsung_dsim_probe(struct platform_device *pdev)
if (IS_ERR(dsi->clks[i])) {
if (strcmp(clk_names[i], "sclk_mipi") == 0) {
dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME);
- if (!IS_ERR(dsi->clks[i]))
+ if (!IS_ERR(dsi->clks[i])) {
+ dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
continue;
+ }
}

dev_info(dev, "failed to get the clock: %s\n", clk_names[i]);
return PTR_ERR(dsi->clks[i]);
}
+
+ if (strcmp(clk_names[i], "sclk_mipi") == 0)
+ dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
}

dsi->reg_base = devm_platform_ioremap_resource(pdev, 0);
--
2.39.2

2023-04-15 10:42:23

by Adam Ford

Subject: [PATCH 4/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing

NXP uses a lookup table to determine the various values for
the PHY Timing based on the clock rate in their downstream
kernel. Since the input clock can be variable, the phy
settings 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.

Signed-off-by: Adam Ford <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 85 +++++++--
drivers/gpu/drm/bridge/samsung-dsim.h | 254 ++++++++++++++++++++++++++
include/drm/bridge/samsung-dsim.h | 1 +
3 files changed, 326 insertions(+), 14 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.h

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 73f0c3fbbdf5..c48db27adafe 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -18,13 +18,14 @@
#include <linux/media-bus-format.h>
#include <linux/of_device.h>
#include <linux/phy/phy.h>
-
+#include <linux/bsearch.h>
#include <video/mipi_display.h>
-
#include <drm/bridge/samsung-dsim.h>
#include <drm/drm_panel.h>
#include <drm/drm_print.h>

+#include "samsung-dsim.h"
+
/* returns true iff both arguments logically differs */
#define NEQV(a, b) (!(a) ^ !(b))

@@ -488,6 +489,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 *
@@ -694,18 +696,52 @@ static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
return 0;
}

+static inline int dphy_timing_default_cmp(const void *key, const void *elt)
+{
+ const struct sec_mipi_dsim_dphy_timing *_key = key;
+ const struct sec_mipi_dsim_dphy_timing *_elt = elt;
+
+ /*
+ * find an element whose 'bit_clk' is equal to
+ * the key's 'bit_clk' value or, the difference
+ * between them is less than 5.
+ */
+
+ if (abs((int)(_elt->bit_clk - _key->bit_clk)) <= 5)
+ return 0;
+
+ if (_key->bit_clk < _elt->bit_clk)
+ /* search bottom half */
+ return 1;
+ else
+ /* search top half */
+ return -1;
+}
+
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;
-
- if (driver_data->has_freqband)
- return;
+ u32 reg = 0;
+ struct drm_display_mode *m = &dsi->mode;
+ struct sec_mipi_dsim_dphy_timing key = { 0 };
+ const struct sec_mipi_dsim_dphy_timing *match = NULL;
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ /* Only dynamic dphy uses the lookup table to determine rates based on clock */
+ if (driver_data->dynamic_dphy) {
+ key.bit_clk = DIV_ROUND_UP(m->clock * bpp, dsi->lanes * 1000);
+
+ match = bsearch(&key, dphy_timing_ln14lpp_v1p2,
+ ARRAY_SIZE(dphy_timing_ln14lpp_v1p2),
+ sizeof(struct sec_mipi_dsim_dphy_timing),
+ dphy_timing_default_cmp);
+ }

/* 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);

/*
@@ -713,7 +749,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(match->lpx) | DSIM_PHYTIMING_HS_EXIT(match->hs_exit);
+ else
+ reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
+
samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);

/*
@@ -729,10 +769,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(match->clk_prepare) |
+ DSIM_PHYTIMING1_CLK_ZERO(match->clk_zero) |
+ DSIM_PHYTIMING1_CLK_POST(match->clk_post) |
+ DSIM_PHYTIMING1_CLK_TRAIL(match->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);

@@ -745,8 +792,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(match->hs_prepare) |
+ DSIM_PHYTIMING2_HS_ZERO(match->hs_zero) |
+ DSIM_PHYTIMING2_HS_TRAIL(match->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);
}

@@ -1353,7 +1409,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/drivers/gpu/drm/bridge/samsung-dsim.h b/drivers/gpu/drm/bridge/samsung-dsim.h
new file mode 100644
index 000000000000..7a382a758cab
--- /dev/null
+++ b/drivers/gpu/drm/bridge/samsung-dsim.h
@@ -0,0 +1,254 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/*
+ * Copyright 2018 NXP
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __SEC_DSIM_DPHY_LN14LPP_H__
+#define __SEC_DSIM_DPHY_LN14LPP_H__
+
+/* DPHY timings structure */
+struct sec_mipi_dsim_dphy_timing {
+ uint32_t bit_clk; /* MHz */
+ uint32_t clk_prepare;
+ uint32_t clk_zero;
+ uint32_t clk_post;
+ uint32_t clk_trail;
+ uint32_t hs_prepare;
+ uint32_t hs_zero;
+ uint32_t hs_trail;
+ uint32_t lpx;
+ uint32_t hs_exit;
+};
+
+#define DSIM_DPHY_TIMING(bclk, cpre, czero, cpost, ctrail, \
+ hpre, hzero, htrail, lp, hexit) \
+ .bit_clk = bclk, \
+ .clk_prepare = cpre, \
+ .clk_zero = czero, \
+ .clk_post = cpost, \
+ .clk_trail = ctrail, \
+ .hs_prepare = hpre, \
+ .hs_zero = hzero, \
+ .hs_trail = htrail, \
+ .lpx = lp, \
+ .hs_exit = hexit
+
+/* descending order based on 'bit_clk' value */
+static const struct sec_mipi_dsim_dphy_timing dphy_timing_ln14lpp_v1p2[] = {
+ { DSIM_DPHY_TIMING(2100, 19, 91, 22, 19, 20, 35, 22, 15, 26), },
+ { DSIM_DPHY_TIMING(2090, 19, 91, 22, 19, 19, 35, 22, 15, 26), },
+ { DSIM_DPHY_TIMING(2080, 19, 91, 21, 18, 19, 35, 22, 15, 26), },
+ { DSIM_DPHY_TIMING(2070, 18, 90, 21, 18, 19, 35, 22, 15, 25), },
+ { DSIM_DPHY_TIMING(2060, 18, 90, 21, 18, 19, 34, 22, 15, 25), },
+ { DSIM_DPHY_TIMING(2050, 18, 89, 21, 18, 19, 34, 22, 15, 25), },
+ { DSIM_DPHY_TIMING(2040, 18, 89, 21, 18, 19, 34, 21, 15, 25), },
+ { DSIM_DPHY_TIMING(2030, 18, 88, 21, 18, 19, 34, 21, 15, 25), },
+ { DSIM_DPHY_TIMING(2020, 18, 88, 21, 18, 19, 34, 21, 15, 25), },
+ { DSIM_DPHY_TIMING(2010, 18, 87, 21, 18, 19, 34, 21, 15, 25), },
+ { DSIM_DPHY_TIMING(2000, 18, 87, 21, 18, 19, 33, 21, 15, 25), },
+ { DSIM_DPHY_TIMING(1990, 18, 87, 21, 18, 18, 33, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1980, 18, 86, 21, 18, 18, 33, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1970, 17, 86, 21, 17, 18, 33, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1960, 17, 85, 21, 17, 18, 33, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1950, 17, 85, 21, 17, 18, 32, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1940, 17, 84, 20, 17, 18, 32, 21, 14, 24), },
+ { DSIM_DPHY_TIMING(1930, 17, 84, 20, 17, 18, 32, 20, 14, 24), },
+ { DSIM_DPHY_TIMING(1920, 17, 84, 20, 17, 18, 32, 20, 14, 24), },
+ { DSIM_DPHY_TIMING(1910, 17, 83, 20, 17, 18, 32, 20, 14, 23), },
+ { DSIM_DPHY_TIMING(1900, 17, 83, 20, 17, 18, 32, 20, 14, 23), },
+ { DSIM_DPHY_TIMING(1890, 17, 82, 20, 17, 18, 31, 20, 14, 23), },
+ { DSIM_DPHY_TIMING(1880, 17, 82, 20, 17, 17, 31, 20, 14, 23), },
+ { DSIM_DPHY_TIMING(1870, 17, 81, 20, 17, 17, 31, 20, 14, 23), },
+ { DSIM_DPHY_TIMING(1860, 16, 81, 20, 17, 17, 31, 20, 13, 23), },
+ { DSIM_DPHY_TIMING(1850, 16, 80, 20, 16, 17, 31, 20, 13, 23), },
+ { DSIM_DPHY_TIMING(1840, 16, 80, 20, 16, 17, 30, 20, 13, 23), },
+ { DSIM_DPHY_TIMING(1830, 16, 80, 20, 16, 17, 30, 20, 13, 22), },
+ { DSIM_DPHY_TIMING(1820, 16, 79, 20, 16, 17, 30, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1810, 16, 79, 19, 16, 17, 30, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1800, 16, 78, 19, 16, 17, 30, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1790, 16, 78, 19, 16, 17, 30, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1780, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1770, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1760, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
+ { DSIM_DPHY_TIMING(1750, 15, 76, 19, 16, 16, 29, 19, 13, 21), },
+ { DSIM_DPHY_TIMING(1740, 15, 76, 19, 15, 16, 29, 19, 13, 21), },
+ { DSIM_DPHY_TIMING(1730, 15, 75, 19, 15, 16, 28, 19, 12, 21), },
+ { DSIM_DPHY_TIMING(1720, 15, 75, 19, 15, 16, 28, 19, 12, 21), },
+ { DSIM_DPHY_TIMING(1710, 15, 74, 19, 15, 16, 28, 18, 12, 21), },
+ { DSIM_DPHY_TIMING(1700, 15, 74, 19, 15, 16, 28, 18, 12, 21), },
+ { DSIM_DPHY_TIMING(1690, 15, 73, 19, 15, 16, 28, 18, 12, 21), },
+ { DSIM_DPHY_TIMING(1680, 15, 73, 18, 15, 16, 28, 18, 12, 21), },
+ { DSIM_DPHY_TIMING(1670, 15, 73, 18, 15, 15, 27, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1660, 15, 72, 18, 15, 15, 27, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1650, 14, 72, 18, 15, 15, 27, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1640, 14, 71, 18, 15, 15, 27, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1630, 14, 71, 18, 15, 15, 27, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1620, 14, 70, 18, 14, 15, 26, 18, 12, 20), },
+ { DSIM_DPHY_TIMING(1610, 14, 70, 18, 14, 15, 26, 17, 12, 20), },
+ { DSIM_DPHY_TIMING(1600, 14, 70, 18, 14, 15, 26, 17, 12, 20), },
+ { DSIM_DPHY_TIMING(1590, 14, 69, 18, 14, 15, 26, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1580, 14, 69, 18, 14, 15, 26, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1570, 14, 68, 18, 14, 15, 26, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1560, 14, 68, 18, 14, 14, 25, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1550, 14, 67, 18, 14, 14, 25, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1540, 13, 67, 17, 14, 14, 25, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1530, 13, 66, 17, 14, 14, 25, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1520, 13, 66, 17, 14, 14, 25, 17, 11, 19), },
+ { DSIM_DPHY_TIMING(1510, 13, 66, 17, 13, 14, 24, 17, 11, 18), },
+ { DSIM_DPHY_TIMING(1500, 13, 65, 17, 13, 14, 24, 16, 11, 18), },
+ { DSIM_DPHY_TIMING(1490, 13, 65, 17, 13, 14, 24, 16, 11, 18), },
+ { DSIM_DPHY_TIMING(1480, 13, 64, 17, 13, 14, 24, 16, 11, 18), },
+ { DSIM_DPHY_TIMING(1470, 13, 64, 17, 13, 14, 24, 16, 11, 18), },
+ { DSIM_DPHY_TIMING(1460, 13, 63, 17, 13, 13, 24, 16, 10, 18), },
+ { DSIM_DPHY_TIMING(1450, 13, 63, 17, 13, 13, 23, 16, 10, 18), },
+ { DSIM_DPHY_TIMING(1440, 13, 63, 17, 13, 13, 23, 16, 10, 18), },
+ { DSIM_DPHY_TIMING(1430, 12, 62, 17, 13, 13, 23, 16, 10, 17), },
+ { DSIM_DPHY_TIMING(1420, 12, 62, 17, 13, 13, 23, 16, 10, 17), },
+ { DSIM_DPHY_TIMING(1410, 12, 61, 16, 13, 13, 23, 16, 10, 17), },
+ { DSIM_DPHY_TIMING(1400, 12, 61, 16, 13, 13, 23, 16, 10, 17), },
+ { DSIM_DPHY_TIMING(1390, 12, 60, 16, 12, 13, 22, 15, 10, 17), },
+ { DSIM_DPHY_TIMING(1380, 12, 60, 16, 12, 13, 22, 15, 10, 17), },
+ { DSIM_DPHY_TIMING(1370, 12, 59, 16, 12, 13, 22, 15, 10, 17), },
+ { DSIM_DPHY_TIMING(1360, 12, 59, 16, 12, 13, 22, 15, 10, 17), },
+ { DSIM_DPHY_TIMING(1350, 12, 59, 16, 12, 12, 22, 15, 10, 16), },
+ { DSIM_DPHY_TIMING(1340, 12, 58, 16, 12, 12, 21, 15, 10, 16), },
+ { DSIM_DPHY_TIMING(1330, 11, 58, 16, 12, 12, 21, 15, 9, 16), },
+ { DSIM_DPHY_TIMING(1320, 11, 57, 16, 12, 12, 21, 15, 9, 16), },
+ { DSIM_DPHY_TIMING(1310, 11, 57, 16, 12, 12, 21, 15, 9, 16), },
+ { DSIM_DPHY_TIMING(1300, 11, 56, 16, 12, 12, 21, 15, 9, 16), },
+ { DSIM_DPHY_TIMING(1290, 11, 56, 16, 12, 12, 21, 15, 9, 16), },
+ { DSIM_DPHY_TIMING(1280, 11, 56, 15, 11, 12, 20, 14, 9, 16), },
+ { DSIM_DPHY_TIMING(1270, 11, 55, 15, 11, 12, 20, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1260, 11, 55, 15, 11, 12, 20, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1250, 11, 54, 15, 11, 11, 20, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1240, 11, 54, 15, 11, 11, 20, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1230, 11, 53, 15, 11, 11, 19, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1220, 10, 53, 15, 11, 11, 19, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1210, 10, 52, 15, 11, 11, 19, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1200, 10, 52, 15, 11, 11, 19, 14, 9, 15), },
+ { DSIM_DPHY_TIMING(1190, 10, 52, 15, 11, 11, 19, 14, 8, 14), },
+ { DSIM_DPHY_TIMING(1180, 10, 51, 15, 11, 11, 19, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1170, 10, 51, 15, 10, 11, 18, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1160, 10, 50, 15, 10, 11, 18, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1150, 10, 50, 15, 10, 11, 18, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1140, 10, 49, 14, 10, 10, 18, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1130, 10, 49, 14, 10, 10, 18, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1120, 10, 49, 14, 10, 10, 17, 13, 8, 14), },
+ { DSIM_DPHY_TIMING(1110, 9, 48, 14, 10, 10, 17, 13, 8, 13), },
+ { DSIM_DPHY_TIMING(1100, 9, 48, 14, 10, 10, 17, 13, 8, 13), },
+ { DSIM_DPHY_TIMING(1090, 9, 47, 14, 10, 10, 17, 13, 8, 13), },
+ { DSIM_DPHY_TIMING(1080, 9, 47, 14, 10, 10, 17, 13, 8, 13), },
+ { DSIM_DPHY_TIMING(1070, 9, 46, 14, 10, 10, 17, 12, 8, 13), },
+ { DSIM_DPHY_TIMING(1060, 9, 46, 14, 10, 10, 16, 12, 7, 13), },
+ { DSIM_DPHY_TIMING(1050, 9, 45, 14, 9, 10, 16, 12, 7, 13), },
+ { DSIM_DPHY_TIMING(1040, 9, 45, 14, 9, 10, 16, 12, 7, 13), },
+ { DSIM_DPHY_TIMING(1030, 9, 45, 14, 9, 9, 16, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(1020, 9, 44, 14, 9, 9, 16, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(1010, 8, 44, 13, 9, 9, 15, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(1000, 8, 43, 13, 9, 9, 15, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(990, 8, 43, 13, 9, 9, 15, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(980, 8, 42, 13, 9, 9, 15, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(970, 8, 42, 13, 9, 9, 15, 12, 7, 12), },
+ { DSIM_DPHY_TIMING(960, 8, 42, 13, 9, 9, 15, 11, 7, 12), },
+ { DSIM_DPHY_TIMING(950, 8, 41, 13, 9, 9, 14, 11, 7, 11), },
+ { DSIM_DPHY_TIMING(940, 8, 41, 13, 8, 9, 14, 11, 7, 11), },
+ { DSIM_DPHY_TIMING(930, 8, 40, 13, 8, 8, 14, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(920, 8, 40, 13, 8, 8, 14, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(910, 8, 39, 13, 8, 8, 14, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(900, 7, 39, 13, 8, 8, 13, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(890, 7, 38, 13, 8, 8, 13, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(880, 7, 38, 12, 8, 8, 13, 11, 6, 11), },
+ { DSIM_DPHY_TIMING(870, 7, 38, 12, 8, 8, 13, 11, 6, 10), },
+ { DSIM_DPHY_TIMING(860, 7, 37, 12, 8, 8, 13, 11, 6, 10), },
+ { DSIM_DPHY_TIMING(850, 7, 37, 12, 8, 8, 13, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(840, 7, 36, 12, 8, 8, 12, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(830, 7, 36, 12, 8, 8, 12, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(820, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(810, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(800, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
+ { DSIM_DPHY_TIMING(790, 6, 34, 12, 7, 7, 11, 10, 5, 9), },
+ { DSIM_DPHY_TIMING(780, 6, 34, 12, 7, 7, 11, 10, 5, 9), },
+ { DSIM_DPHY_TIMING(770, 6, 33, 12, 7, 7, 11, 10, 5, 9), },
+ { DSIM_DPHY_TIMING(760, 6, 33, 12, 7, 7, 11, 10, 5, 9), },
+ { DSIM_DPHY_TIMING(750, 6, 32, 12, 7, 7, 11, 9, 5, 9), },
+ { DSIM_DPHY_TIMING(740, 6, 32, 11, 7, 7, 11, 9, 5, 9), },
+ { DSIM_DPHY_TIMING(730, 6, 31, 11, 7, 7, 10, 9, 5, 9), },
+ { DSIM_DPHY_TIMING(720, 6, 31, 11, 7, 6, 10, 9, 5, 9), },
+ { DSIM_DPHY_TIMING(710, 6, 31, 11, 6, 6, 10, 9, 5, 8), },
+ { DSIM_DPHY_TIMING(700, 6, 30, 11, 6, 6, 10, 9, 5, 8), },
+ { DSIM_DPHY_TIMING(690, 5, 30, 11, 6, 6, 10, 9, 5, 8), },
+ { DSIM_DPHY_TIMING(680, 5, 29, 11, 6, 6, 9, 9, 5, 8), },
+ { DSIM_DPHY_TIMING(670, 5, 29, 11, 6, 6, 9, 9, 5, 8), },
+ { DSIM_DPHY_TIMING(660, 5, 28, 11, 6, 6, 9, 9, 4, 8), },
+ { DSIM_DPHY_TIMING(650, 5, 28, 11, 6, 6, 9, 9, 4, 8), },
+ { DSIM_DPHY_TIMING(640, 5, 28, 11, 6, 6, 9, 8, 4, 8), },
+ { DSIM_DPHY_TIMING(630, 5, 27, 11, 6, 6, 9, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(620, 5, 27, 11, 6, 6, 8, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(610, 5, 26, 10, 6, 5, 8, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(600, 5, 26, 10, 6, 5, 8, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(590, 5, 25, 10, 5, 5, 8, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(580, 4, 25, 10, 5, 5, 8, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(570, 4, 24, 10, 5, 5, 7, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(560, 4, 24, 10, 5, 5, 7, 8, 4, 7), },
+ { DSIM_DPHY_TIMING(550, 4, 24, 10, 5, 5, 7, 8, 4, 6), },
+ { DSIM_DPHY_TIMING(540, 4, 23, 10, 5, 5, 7, 8, 4, 6), },
+ { DSIM_DPHY_TIMING(530, 4, 23, 10, 5, 5, 7, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(520, 4, 22, 10, 5, 5, 7, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(510, 4, 22, 10, 5, 5, 6, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(500, 4, 21, 10, 5, 4, 6, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(490, 4, 21, 10, 5, 4, 6, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(480, 4, 21, 9, 4, 4, 6, 7, 3, 6), },
+ { DSIM_DPHY_TIMING(470, 3, 20, 9, 4, 4, 6, 7, 3, 5), },
+ { DSIM_DPHY_TIMING(460, 3, 20, 9, 4, 4, 5, 7, 3, 5), },
+ { DSIM_DPHY_TIMING(450, 3, 19, 9, 4, 4, 5, 7, 3, 5), },
+ { DSIM_DPHY_TIMING(440, 3, 19, 9, 4, 4, 5, 7, 3, 5), },
+ { DSIM_DPHY_TIMING(430, 3, 18, 9, 4, 4, 5, 7, 3, 5), },
+ { DSIM_DPHY_TIMING(420, 3, 18, 9, 4, 4, 5, 6, 3, 5), },
+ { DSIM_DPHY_TIMING(410, 3, 17, 9, 4, 4, 5, 6, 3, 5), },
+ { DSIM_DPHY_TIMING(400, 3, 17, 9, 4, 3, 4, 6, 3, 5), },
+ { DSIM_DPHY_TIMING(390, 3, 17, 9, 4, 3, 4, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(380, 3, 16, 9, 4, 3, 4, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(370, 2, 16, 9, 3, 3, 4, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(360, 2, 15, 9, 3, 3, 4, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(350, 2, 15, 9, 3, 3, 3, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(340, 2, 14, 8, 3, 3, 3, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(330, 2, 14, 8, 3, 3, 3, 6, 2, 4), },
+ { DSIM_DPHY_TIMING(320, 2, 14, 8, 3, 3, 3, 5, 2, 4), },
+ { DSIM_DPHY_TIMING(310, 2, 13, 8, 3, 3, 3, 5, 2, 3), },
+ { DSIM_DPHY_TIMING(300, 2, 13, 8, 3, 3, 3, 5, 2, 3), },
+ { DSIM_DPHY_TIMING(290, 2, 12, 8, 3, 2, 2, 5, 2, 3), },
+ { DSIM_DPHY_TIMING(280, 2, 12, 8, 3, 2, 2, 5, 2, 3), },
+ { DSIM_DPHY_TIMING(270, 2, 11, 8, 3, 2, 2, 5, 2, 3), },
+ { DSIM_DPHY_TIMING(260, 1, 11, 8, 3, 2, 2, 5, 1, 3), },
+ { DSIM_DPHY_TIMING(250, 1, 10, 8, 2, 2, 2, 5, 1, 3), },
+ { DSIM_DPHY_TIMING(240, 1, 9, 8, 2, 2, 1, 5, 1, 3), },
+ { DSIM_DPHY_TIMING(230, 1, 8, 8, 2, 2, 1, 5, 1, 2), },
+ { DSIM_DPHY_TIMING(220, 1, 8, 8, 2, 2, 1, 5, 1, 2), },
+ { DSIM_DPHY_TIMING(210, 1, 7, 7, 2, 2, 1, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(200, 1, 7, 7, 2, 2, 1, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(190, 1, 7, 7, 2, 1, 1, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(180, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(170, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(160, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
+ { DSIM_DPHY_TIMING(150, 0, 5, 7, 2, 1, 0, 4, 1, 1), },
+ { DSIM_DPHY_TIMING(140, 0, 5, 7, 1, 1, 0, 4, 1, 1), },
+ { DSIM_DPHY_TIMING(130, 0, 4, 7, 1, 1, 0, 4, 0, 1), },
+ { DSIM_DPHY_TIMING(120, 0, 4, 7, 1, 1, 0, 4, 0, 1), },
+ { DSIM_DPHY_TIMING(110, 0, 3, 7, 1, 0, 0, 4, 0, 1), },
+ { DSIM_DPHY_TIMING(100, 0, 3, 7, 1, 0, 0, 3, 0, 1), },
+ { DSIM_DPHY_TIMING(90, 0, 2, 7, 1, 0, 0, 3, 0, 1), },
+ { DSIM_DPHY_TIMING(80, 0, 2, 6, 1, 0, 0, 3, 0, 1), },
+};
+
+#endif
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

2023-04-15 10:42:27

by Adam Ford

Subject: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

The high-speed clock is hard-coded to the burst-clock
frequency specified in the device tree. However, when
using devices like certain bridge chips without burst mode
and varying resolutions and refresh rates, it may be
necessary to set the high-speed clock dynamically based
on the desired pixel clock for the connected device.

Signed-off-by: Adam Ford <[email protected]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index c48db27adafe..5aa3a44f15ec 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -659,11 +659,21 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,

static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
{
- unsigned long hs_clk, byte_clk, esc_clk;
+ unsigned long hs_clk, byte_clk, esc_clk, pix_clk;
unsigned long esc_div;
u32 reg;
+ struct drm_display_mode *m = &dsi->mode;
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ /* m->clock is in KHz */
+ pix_clk = m->clock * 1000;
+
+ /* Use burst_clk_rate for burst mode, otherwise use the pix_clk */
+ if ((dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) && dsi->burst_clk_rate)
+ hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
+ else
+ hs_clk = samsung_dsim_set_pll(dsi, DIV_ROUND_UP(pix_clk * bpp, dsi->lanes));

- hs_clk = samsung_dsim_set_pll(dsi, dsi->burst_clk_rate);
if (!hs_clk) {
dev_err(dsi->dev, "failed to configure DSI PLL\n");
return -EFAULT;
--
2.39.2

2023-04-15 10:42:38

by Adam Ford

Subject: [PATCH 6/6] arm64: dts: imx8mn: Fix video clock parents

There are a few clocks whose parents are set in mipi_dsi
and mxsfb nodes, but these clocks are used by the disp_blk_ctrl
power domain which may cause an issue when re-parenting, resuling
in a disp_pixel clock having the wrong parent and wrong rate.

Fix this by moving the assigned-clock-parents as associate clock
assignments to the power-domain node to setup these clocks before
they are enabled.

Fixes: d825fb6455d5 ("arm64: dts: imx8mn: Add display pipeline components")
Signed-off-by: Adam Ford <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mn.dtsi | 28 ++++++++++++-----------
1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index bd84db550053..8be8f090e8b8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -1069,13 +1069,6 @@ lcdif: lcdif@32e00000 {
<&clk IMX8MN_CLK_DISP_APB_ROOT>,
<&clk IMX8MN_CLK_DISP_AXI_ROOT>;
clock-names = "pix", "axi", "disp_axi";
- assigned-clocks = <&clk IMX8MN_CLK_DISP_PIXEL_ROOT>,
- <&clk IMX8MN_CLK_DISP_AXI>,
- <&clk IMX8MN_CLK_DISP_APB>;
- assigned-clock-parents = <&clk IMX8MN_CLK_DISP_PIXEL>,
- <&clk IMX8MN_SYS_PLL2_1000M>,
- <&clk IMX8MN_SYS_PLL1_800M>;
- assigned-clock-rates = <594000000>, <500000000>, <200000000>;
interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
power-domains = <&disp_blk_ctrl IMX8MN_DISPBLK_PD_LCDIF>;
status = "disabled";
@@ -1093,12 +1086,6 @@ mipi_dsi: dsi@32e10000 {
clocks = <&clk IMX8MN_CLK_DSI_CORE>,
<&clk IMX8MN_CLK_DSI_PHY_REF>;
clock-names = "bus_clk", "sclk_mipi";
- assigned-clocks = <&clk IMX8MN_CLK_DSI_CORE>,
- <&clk IMX8MN_CLK_DSI_PHY_REF>;
- assigned-clock-parents = <&clk IMX8MN_SYS_PLL1_266M>,
- <&clk IMX8MN_CLK_24M>;
- assigned-clock-rates = <266000000>, <24000000>;
- samsung,pll-clock-frequency = <24000000>;
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
power-domains = <&disp_blk_ctrl IMX8MN_DISPBLK_PD_MIPI_DSI>;
status = "disabled";
@@ -1142,6 +1129,21 @@ disp_blk_ctrl: blk-ctrl@32e28000 {
"lcdif-axi", "lcdif-apb", "lcdif-pix",
"dsi-pclk", "dsi-ref",
"csi-aclk", "csi-pclk";
+ assigned-clocks = <&clk IMX8MN_CLK_DSI_CORE>,
+ <&clk IMX8MN_CLK_DSI_PHY_REF>,
+ <&clk IMX8MN_CLK_DISP_PIXEL>,
+ <&clk IMX8MN_CLK_DISP_AXI>,
+ <&clk IMX8MN_CLK_DISP_APB>;
+ assigned-clock-parents = <&clk IMX8MN_SYS_PLL1_266M>,
+ <&clk IMX8MN_CLK_24M>,
+ <&clk IMX8MN_VIDEO_PLL1_OUT>,
+ <&clk IMX8MN_SYS_PLL2_1000M>,
+ <&clk IMX8MN_SYS_PLL1_800M>;
+ assigned-clock-rates = <266000000>,
+ <24000000>,
+ <594000000>,
+ <500000000>,
+ <200000000>;
#power-domain-cells = <1>;
};

--
2.39.2

2023-04-16 22:15:46

by Marek Vasut

Subject: Re: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

On 4/15/23 12:40, Adam Ford wrote:
> According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
> and max values for M and the frequency range for the VCO_out
> calculator were incorrect. This also appears to be the case for the
> imx8mn and imx8mp.
>
> To fix this, make new variables to hold the min and max values of m
> and the minimum value of VCO_out, and update the PMS calculator to
> use these new variables instead of using hard-coded values to keep
> the backwards compatibility with other parts using this driver.

[...]

> static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> @@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> */
> .pll_p_offset = 14,
> .reg_values = imx8mm_dsim_reg_values,
> + .m_min = 64,
> + .m_max = 1023,
> + .vco_min = 1050,

You might want to call this 'min_freq' since there is a 'max_freq' which
seems to indicate what VCO max frequency is.

Note that the same datasheet contains the following information:
"
MIPI_DPHY_M_PLLPMS field descriptions

12–4 PMS_M
Specifies the PLL PMS value for the M divider
NOTE: The programmable divider range should be within 25 to 125 to
ensure PLL stability.
NOTE: The M and P divider values should be considered together to ensure
VCO ouput frequency
(VCO_out) range is between 350 MHz to 750 MHz.
Please refer to the topic DPHY PLL for more information.
"

2023-04-16 22:30:13

by Marek Vasut

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On 4/15/23 12:41, Adam Ford wrote:
> Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> having an entry in the device tree for samsung,pll-clock-frequency.
>
> Signed-off-by: Adam Ford <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 9fec32b44e05..73f0c3fbbdf5 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> struct device_node *node = dev->of_node;
> int ret;
>
> - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> - &dsi->pll_clk_rate);
> - if (ret < 0)
> - return ret;
> -
> ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> &dsi->burst_clk_rate);
> if (ret < 0)

Does this break compatibility with old samsung DTs ?

2023-04-16 22:32:02

by Marek Vasut

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On 4/15/23 12:40, Adam Ford wrote:
> If there is more than one lane, the HFP, HBP, and HSA is calculated in
> bytes/pixel, then they are divided amongst the different lanes with some
> additional overhead. This is necessary to achieve higher resolutions while
> keeping the pixel clocks lower as the number of lanes increase.
>
> Signed-off-by: Adam Ford <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e0a402a85787..1ccbad4ea577 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -215,6 +215,7 @@
> #define DSI_RX_FIFO_SIZE 256
> #define DSI_XFER_TIMEOUT_MS 100
> #define DSI_RX_FIFO_EMPTY 0x30800002
> +#define DSI_HSYNC_PKT_OVERHEAD 6
>
> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
>
> @@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> | DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
> samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);
>
> - reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> - | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> - samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> + /*
> + * If there is more than one lane, the HFP, HBP, and HSA
> + * is calculated in bytes/pixel, then they are divided
> + * amongst the different lanes with some additional
> + * overhead correction
> + */

Did you find any confirmation of this in the MX8M* datasheet or at least
by measuring the DSI data lanes with a scope ?

It would be real cool if this could be confirmed somehow, and we could
rule out that this tweaking of HSA/HSE/... stuff isn't related to either
LP-HS transition timing calculation this driver is missing, OR,
incorrect flags in various bridge/panel drivers like commit:

ca161b259cc84 ("drm/bridge: ti-sn65dsi83: Do not generate HFP/HBP/HSA
and EOT packet")

2023-04-16 22:39:30

by Marek Vasut

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On 4/15/23 12:41, Adam Ford wrote:
> The high-speed clock is hard-coded to the burst-clock
> frequency specified in the device tree. However, when
> using devices like certain bridge chips without burst mode
> and varying resolutions and refresh rates, it may be
> necessary to set the high-speed clock dynamically based
> on the desired pixel clock for the connected device.

The link rate negotiation should happen internally between the nearest
bridge and DSIM, so please add that to DRM core instead of hacking
around it by tweaking the HS clock again.

2023-04-16 23:24:56

by Adam Ford

Subject: Re: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

On Sun, Apr 16, 2023 at 5:07 PM Marek Vasut <[email protected]> wrote:
>
> On 4/15/23 12:40, Adam Ford wrote:
> > According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
> > and max values for M and the frequency range for the VCO_out
> > calculator were incorrect. This also appears to be the case for the
> > imx8mn and imx8mp.
> >
> > To fix this, make new variables to hold the min and max values of m
> > and the minimum value of VCO_out, and update the PMS calculator to
> > use these new variables instead of using hard-coded values to keep
> > the backwards compatibility with other parts using this driver.
>
> [...]
>
> > static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> > @@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> > */
> > .pll_p_offset = 14,
> > .reg_values = imx8mm_dsim_reg_values,
> > + .m_min = 64,
> > + .m_max = 1023,
> > + .vco_min = 1050,
>
> You might want to call this 'min_freq' since there is a 'max_freq' which
> seems to indicate what VCO max frequency is.
>
> Note that the same datasheet contains the following information:
> "
> MIPI_DPHY_M_PLLPMS field descriptions
>
> 12–4 PMS_M
> Specifies the PLL PMS value for the M divider
> NOTE: The programmable divider range should be within 25 to 125 to
> ensure PLL stability.

I was confused by this because this statement is not consistent with
the link they reference jumps me to the table where it reads M is
between 64 and 1023.

> NOTE: The M and P divider values should be considered together to ensure
> VCO ouput frequency
> (VCO_out) range is between 350 MHz to 750 MHz.
> Please refer to the topic DPHY PLL for more information.

I was confused by this too, because the NXP documentation reads the
350 - 750MHz that you state, but "Table 13-45: DPHY PLL Parameters"
which immediately follows that sentence on page 4158 shows VCO_out is
between 1050-2100 MHz.

I compared the PMS values for a variety of frequencies to those that
were set in the downstream NXP code, and the PMS values matched.
Maybe someone from NXP can explain the discrepancy.

adam

> "

2023-04-16 23:40:23

by Marek Vasut

Subject: Re: [PATCH 4/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing

On 4/15/23 12:41, Adam Ford wrote:
> NXP uses a lookup table to determine the various values for
> the PHY Timing based on the clock rate in their downstream
> kernel. Since the input clock can be variable, the phy
> settings 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.

Isn't there already a generic LP2HS transition time calculation in the
kernel ?

This looks like a generic calculation which should be done by all the
DSI hosts, so why not introduce a generic helper to do such a
calculation (and not a table please) ?

2023-04-17 07:08:56

by Alexander Stein

Subject: Re: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

Hi,

Am Montag, 17. April 2023, 00:31:24 CEST schrieb Adam Ford:
> On Sun, Apr 16, 2023 at 5:07 PM Marek Vasut <[email protected]> wrote:
> > On 4/15/23 12:40, Adam Ford wrote:
> > > According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
> > > and max values for M and the frequency range for the VCO_out
> > > calculator were incorrect. This also appears to be the case for the
> > > imx8mn and imx8mp.
> > >
> > > To fix this, make new variables to hold the min and max values of m
> > > and the minimum value of VCO_out, and update the PMS calculator to
> > > use these new variables instead of using hard-coded values to keep
> > > the backwards compatibility with other parts using this driver.
> >
> > [...]
> >
> > > static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data =
> > > {
> > >
> > > @@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data
> > > imx8mm_dsi_driver_data = {> >
> > > */
> > >
> > > .pll_p_offset = 14,
> > > .reg_values = imx8mm_dsim_reg_values,
> > >
> > > + .m_min = 64,
> > > + .m_max = 1023,
> > > + .vco_min = 1050,
> >
> > You might want to call this 'min_freq' since there is a 'max_freq' which
> > seems to indicate what VCO max frequency is.
> >
> > Note that the same datasheet contains the following information:
> > "
> > MIPI_DPHY_M_PLLPMS field descriptions
> >
> > 12–4 PMS_M
> > Specifies the PLL PMS value for the M divider
> > NOTE: The programmable divider range should be within 25 to 125 to
> > ensure PLL stability.
>
> I was confused by this because this statement is not consistent with
> the link they reference jumps me to the table where it reads M is
> between 64 and 1023.
>
> > NOTE: The M and P divider values should be considered together to ensure
> > VCO ouput frequency
> > (VCO_out) range is between 350 MHz to 750 MHz.
> > Please refer to the topic DPHY PLL for more information.
>
> I was confused by this too, because the NXP documentation reads the
> 350 - 750MHz that you state, but "Table 13-45: DPHY PLL Parameters"
> which immediately follows that sentence on page 4158 shows VCO_out is
> between 1050-2100 MHz.
>
> I compared the PMS values for a variety of frequencies to those that
> were set in the downstream NXP code, and the PMS values matched.
> Maybe someone from NXP can explain the discrepancy.

Also note that, according to Table 13-28 in RM (Rev 2 07/2022) for i.MX8M
Nano, VCO_out and Fout has a maximum of 1500MHz only. Although the note above
mentions a range of 1050-2100MHz...

Best regards,
Alexander
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

2023-04-17 08:43:41

by Lucas Stach

Subject: Re: [PATCH 4/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing

Hi Adam,

Am Samstag, dem 15.04.2023 um 05:41 -0500 schrieb Adam Ford:
> NXP uses a lookup table to determine the various values for
> the PHY Timing based on the clock rate in their downstream
> kernel. Since the input clock can be variable, the phy
> settings 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.
>

I haven't checked if this generates values close to the ones in this
table, but I guess it should be worth a try to use
phy_mipi_dphy_get_default_config() instead.

Regards,
Lucas

> Signed-off-by: Adam Ford <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 85 +++++++--
> drivers/gpu/drm/bridge/samsung-dsim.h | 254 ++++++++++++++++++++++++++
> include/drm/bridge/samsung-dsim.h | 1 +
> 3 files changed, 326 insertions(+), 14 deletions(-)
> create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.h
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 73f0c3fbbdf5..c48db27adafe 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -18,13 +18,14 @@
> #include <linux/media-bus-format.h>
> #include <linux/of_device.h>
> #include <linux/phy/phy.h>
> -
> +#include <linux/bsearch.h>
> #include <video/mipi_display.h>
> -
> #include <drm/bridge/samsung-dsim.h>
> #include <drm/drm_panel.h>
> #include <drm/drm_print.h>
>
> +#include "samsung-dsim.h"
> +
> /* returns true iff both arguments logically differs */
> #define NEQV(a, b) (!(a) ^ !(b))
>
> @@ -488,6 +489,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 *
> @@ -694,18 +696,52 @@ static int samsung_dsim_enable_clock(struct samsung_dsim *dsi)
> return 0;
> }
>
> +static inline int dphy_timing_default_cmp(const void *key, const void *elt)
> +{
> + const struct sec_mipi_dsim_dphy_timing *_key = key;
> + const struct sec_mipi_dsim_dphy_timing *_elt = elt;
> +
> + /*
> + * find an element whose 'bit_clk' is equal to
> + * the key's 'bit_clk' value or, the difference
> + * between them is less than 5.
> + */
> +
> + if (abs((int)(_elt->bit_clk - _key->bit_clk)) <= 5)
> + return 0;
> +
> + if (_key->bit_clk < _elt->bit_clk)
> + /* search bottom half */
> + return 1;
> + else
> + /* search top half */
> + return -1;
> +}
> +
> 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;
> -
> - if (driver_data->has_freqband)
> - return;
> + u32 reg = 0;
> + struct drm_display_mode *m = &dsi->mode;
> + struct sec_mipi_dsim_dphy_timing key = { 0 };
> + const struct sec_mipi_dsim_dphy_timing *match = NULL;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> +
> + /* Only dynamic dphy uses the lookup table to determine rates based on clock */
> + if (driver_data->dynamic_dphy) {
> + key.bit_clk = DIV_ROUND_UP(m->clock * bpp, dsi->lanes * 1000);
> +
> + match = bsearch(&key, dphy_timing_ln14lpp_v1p2,
> + ARRAY_SIZE(dphy_timing_ln14lpp_v1p2),
> + sizeof(struct sec_mipi_dsim_dphy_timing),
> + dphy_timing_default_cmp);
> + }
>
> /* 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);
>
> /*
> @@ -713,7 +749,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(match->lpx) | DSIM_PHYTIMING_HS_EXIT(match->hs_exit);
> + else
> + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> +
> samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
>
> /*
> @@ -729,10 +769,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(match->clk_prepare) |
> + DSIM_PHYTIMING1_CLK_ZERO(match->clk_zero) |
> + DSIM_PHYTIMING1_CLK_POST(match->clk_post) |
> + DSIM_PHYTIMING1_CLK_TRAIL(match->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);
>
> @@ -745,8 +792,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(match->hs_prepare) |
> + DSIM_PHYTIMING2_HS_ZERO(match->hs_zero) |
> + DSIM_PHYTIMING2_HS_TRAIL(match->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);
> }
>
> @@ -1353,7 +1409,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/drivers/gpu/drm/bridge/samsung-dsim.h b/drivers/gpu/drm/bridge/samsung-dsim.h
> new file mode 100644
> index 000000000000..7a382a758cab
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.h
> @@ -0,0 +1,254 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +/*
> + * Copyright 2018 NXP
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __SEC_DSIM_DPHY_LN14LPP_H__
> +#define __SEC_DSIM_DPHY_LN14LPP_H__
> +
> +/* DPHY timings structure */
> +struct sec_mipi_dsim_dphy_timing {
> + uint32_t bit_clk; /* MHz */
> + uint32_t clk_prepare;
> + uint32_t clk_zero;
> + uint32_t clk_post;
> + uint32_t clk_trail;
> + uint32_t hs_prepare;
> + uint32_t hs_zero;
> + uint32_t hs_trail;
> + uint32_t lpx;
> + uint32_t hs_exit;
> +};
> +
> +#define DSIM_DPHY_TIMING(bclk, cpre, czero, cpost, ctrail, \
> + hpre, hzero, htrail, lp, hexit) \
> + .bit_clk = bclk, \
> + .clk_prepare = cpre, \
> + .clk_zero = czero, \
> + .clk_post = cpost, \
> + .clk_trail = ctrail, \
> + .hs_prepare = hpre, \
> + .hs_zero = hzero, \
> + .hs_trail = htrail, \
> + .lpx = lp, \
> + .hs_exit = hexit
> +
> +/* descending order based on 'bit_clk' value */
> +static const struct sec_mipi_dsim_dphy_timing dphy_timing_ln14lpp_v1p2[] = {
> + { DSIM_DPHY_TIMING(2100, 19, 91, 22, 19, 20, 35, 22, 15, 26), },
> + { DSIM_DPHY_TIMING(2090, 19, 91, 22, 19, 19, 35, 22, 15, 26), },
> + { DSIM_DPHY_TIMING(2080, 19, 91, 21, 18, 19, 35, 22, 15, 26), },
> + { DSIM_DPHY_TIMING(2070, 18, 90, 21, 18, 19, 35, 22, 15, 25), },
> + { DSIM_DPHY_TIMING(2060, 18, 90, 21, 18, 19, 34, 22, 15, 25), },
> + { DSIM_DPHY_TIMING(2050, 18, 89, 21, 18, 19, 34, 22, 15, 25), },
> + { DSIM_DPHY_TIMING(2040, 18, 89, 21, 18, 19, 34, 21, 15, 25), },
> + { DSIM_DPHY_TIMING(2030, 18, 88, 21, 18, 19, 34, 21, 15, 25), },
> + { DSIM_DPHY_TIMING(2020, 18, 88, 21, 18, 19, 34, 21, 15, 25), },
> + { DSIM_DPHY_TIMING(2010, 18, 87, 21, 18, 19, 34, 21, 15, 25), },
> + { DSIM_DPHY_TIMING(2000, 18, 87, 21, 18, 19, 33, 21, 15, 25), },
> + { DSIM_DPHY_TIMING(1990, 18, 87, 21, 18, 18, 33, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1980, 18, 86, 21, 18, 18, 33, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1970, 17, 86, 21, 17, 18, 33, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1960, 17, 85, 21, 17, 18, 33, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1950, 17, 85, 21, 17, 18, 32, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1940, 17, 84, 20, 17, 18, 32, 21, 14, 24), },
> + { DSIM_DPHY_TIMING(1930, 17, 84, 20, 17, 18, 32, 20, 14, 24), },
> + { DSIM_DPHY_TIMING(1920, 17, 84, 20, 17, 18, 32, 20, 14, 24), },
> + { DSIM_DPHY_TIMING(1910, 17, 83, 20, 17, 18, 32, 20, 14, 23), },
> + { DSIM_DPHY_TIMING(1900, 17, 83, 20, 17, 18, 32, 20, 14, 23), },
> + { DSIM_DPHY_TIMING(1890, 17, 82, 20, 17, 18, 31, 20, 14, 23), },
> + { DSIM_DPHY_TIMING(1880, 17, 82, 20, 17, 17, 31, 20, 14, 23), },
> + { DSIM_DPHY_TIMING(1870, 17, 81, 20, 17, 17, 31, 20, 14, 23), },
> + { DSIM_DPHY_TIMING(1860, 16, 81, 20, 17, 17, 31, 20, 13, 23), },
> + { DSIM_DPHY_TIMING(1850, 16, 80, 20, 16, 17, 31, 20, 13, 23), },
> + { DSIM_DPHY_TIMING(1840, 16, 80, 20, 16, 17, 30, 20, 13, 23), },
> + { DSIM_DPHY_TIMING(1830, 16, 80, 20, 16, 17, 30, 20, 13, 22), },
> + { DSIM_DPHY_TIMING(1820, 16, 79, 20, 16, 17, 30, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1810, 16, 79, 19, 16, 17, 30, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1800, 16, 78, 19, 16, 17, 30, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1790, 16, 78, 19, 16, 17, 30, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1780, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1770, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1760, 16, 77, 19, 16, 16, 29, 19, 13, 22), },
> + { DSIM_DPHY_TIMING(1750, 15, 76, 19, 16, 16, 29, 19, 13, 21), },
> + { DSIM_DPHY_TIMING(1740, 15, 76, 19, 15, 16, 29, 19, 13, 21), },
> + { DSIM_DPHY_TIMING(1730, 15, 75, 19, 15, 16, 28, 19, 12, 21), },
> + { DSIM_DPHY_TIMING(1720, 15, 75, 19, 15, 16, 28, 19, 12, 21), },
> + { DSIM_DPHY_TIMING(1710, 15, 74, 19, 15, 16, 28, 18, 12, 21), },
> + { DSIM_DPHY_TIMING(1700, 15, 74, 19, 15, 16, 28, 18, 12, 21), },
> + { DSIM_DPHY_TIMING(1690, 15, 73, 19, 15, 16, 28, 18, 12, 21), },
> + { DSIM_DPHY_TIMING(1680, 15, 73, 18, 15, 16, 28, 18, 12, 21), },
> + { DSIM_DPHY_TIMING(1670, 15, 73, 18, 15, 15, 27, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1660, 15, 72, 18, 15, 15, 27, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1650, 14, 72, 18, 15, 15, 27, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1640, 14, 71, 18, 15, 15, 27, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1630, 14, 71, 18, 15, 15, 27, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1620, 14, 70, 18, 14, 15, 26, 18, 12, 20), },
> + { DSIM_DPHY_TIMING(1610, 14, 70, 18, 14, 15, 26, 17, 12, 20), },
> + { DSIM_DPHY_TIMING(1600, 14, 70, 18, 14, 15, 26, 17, 12, 20), },
> + { DSIM_DPHY_TIMING(1590, 14, 69, 18, 14, 15, 26, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1580, 14, 69, 18, 14, 15, 26, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1570, 14, 68, 18, 14, 15, 26, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1560, 14, 68, 18, 14, 14, 25, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1550, 14, 67, 18, 14, 14, 25, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1540, 13, 67, 17, 14, 14, 25, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1530, 13, 66, 17, 14, 14, 25, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1520, 13, 66, 17, 14, 14, 25, 17, 11, 19), },
> + { DSIM_DPHY_TIMING(1510, 13, 66, 17, 13, 14, 24, 17, 11, 18), },
> + { DSIM_DPHY_TIMING(1500, 13, 65, 17, 13, 14, 24, 16, 11, 18), },
> + { DSIM_DPHY_TIMING(1490, 13, 65, 17, 13, 14, 24, 16, 11, 18), },
> + { DSIM_DPHY_TIMING(1480, 13, 64, 17, 13, 14, 24, 16, 11, 18), },
> + { DSIM_DPHY_TIMING(1470, 13, 64, 17, 13, 14, 24, 16, 11, 18), },
> + { DSIM_DPHY_TIMING(1460, 13, 63, 17, 13, 13, 24, 16, 10, 18), },
> + { DSIM_DPHY_TIMING(1450, 13, 63, 17, 13, 13, 23, 16, 10, 18), },
> + { DSIM_DPHY_TIMING(1440, 13, 63, 17, 13, 13, 23, 16, 10, 18), },
> + { DSIM_DPHY_TIMING(1430, 12, 62, 17, 13, 13, 23, 16, 10, 17), },
> + { DSIM_DPHY_TIMING(1420, 12, 62, 17, 13, 13, 23, 16, 10, 17), },
> + { DSIM_DPHY_TIMING(1410, 12, 61, 16, 13, 13, 23, 16, 10, 17), },
> + { DSIM_DPHY_TIMING(1400, 12, 61, 16, 13, 13, 23, 16, 10, 17), },
> + { DSIM_DPHY_TIMING(1390, 12, 60, 16, 12, 13, 22, 15, 10, 17), },
> + { DSIM_DPHY_TIMING(1380, 12, 60, 16, 12, 13, 22, 15, 10, 17), },
> + { DSIM_DPHY_TIMING(1370, 12, 59, 16, 12, 13, 22, 15, 10, 17), },
> + { DSIM_DPHY_TIMING(1360, 12, 59, 16, 12, 13, 22, 15, 10, 17), },
> + { DSIM_DPHY_TIMING(1350, 12, 59, 16, 12, 12, 22, 15, 10, 16), },
> + { DSIM_DPHY_TIMING(1340, 12, 58, 16, 12, 12, 21, 15, 10, 16), },
> + { DSIM_DPHY_TIMING(1330, 11, 58, 16, 12, 12, 21, 15, 9, 16), },
> + { DSIM_DPHY_TIMING(1320, 11, 57, 16, 12, 12, 21, 15, 9, 16), },
> + { DSIM_DPHY_TIMING(1310, 11, 57, 16, 12, 12, 21, 15, 9, 16), },
> + { DSIM_DPHY_TIMING(1300, 11, 56, 16, 12, 12, 21, 15, 9, 16), },
> + { DSIM_DPHY_TIMING(1290, 11, 56, 16, 12, 12, 21, 15, 9, 16), },
> + { DSIM_DPHY_TIMING(1280, 11, 56, 15, 11, 12, 20, 14, 9, 16), },
> + { DSIM_DPHY_TIMING(1270, 11, 55, 15, 11, 12, 20, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1260, 11, 55, 15, 11, 12, 20, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1250, 11, 54, 15, 11, 11, 20, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1240, 11, 54, 15, 11, 11, 20, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1230, 11, 53, 15, 11, 11, 19, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1220, 10, 53, 15, 11, 11, 19, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1210, 10, 52, 15, 11, 11, 19, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1200, 10, 52, 15, 11, 11, 19, 14, 9, 15), },
> + { DSIM_DPHY_TIMING(1190, 10, 52, 15, 11, 11, 19, 14, 8, 14), },
> + { DSIM_DPHY_TIMING(1180, 10, 51, 15, 11, 11, 19, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1170, 10, 51, 15, 10, 11, 18, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1160, 10, 50, 15, 10, 11, 18, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1150, 10, 50, 15, 10, 11, 18, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1140, 10, 49, 14, 10, 10, 18, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1130, 10, 49, 14, 10, 10, 18, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1120, 10, 49, 14, 10, 10, 17, 13, 8, 14), },
> + { DSIM_DPHY_TIMING(1110, 9, 48, 14, 10, 10, 17, 13, 8, 13), },
> + { DSIM_DPHY_TIMING(1100, 9, 48, 14, 10, 10, 17, 13, 8, 13), },
> + { DSIM_DPHY_TIMING(1090, 9, 47, 14, 10, 10, 17, 13, 8, 13), },
> + { DSIM_DPHY_TIMING(1080, 9, 47, 14, 10, 10, 17, 13, 8, 13), },
> + { DSIM_DPHY_TIMING(1070, 9, 46, 14, 10, 10, 17, 12, 8, 13), },
> + { DSIM_DPHY_TIMING(1060, 9, 46, 14, 10, 10, 16, 12, 7, 13), },
> + { DSIM_DPHY_TIMING(1050, 9, 45, 14, 9, 10, 16, 12, 7, 13), },
> + { DSIM_DPHY_TIMING(1040, 9, 45, 14, 9, 10, 16, 12, 7, 13), },
> + { DSIM_DPHY_TIMING(1030, 9, 45, 14, 9, 9, 16, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(1020, 9, 44, 14, 9, 9, 16, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(1010, 8, 44, 13, 9, 9, 15, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(1000, 8, 43, 13, 9, 9, 15, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(990, 8, 43, 13, 9, 9, 15, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(980, 8, 42, 13, 9, 9, 15, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(970, 8, 42, 13, 9, 9, 15, 12, 7, 12), },
> + { DSIM_DPHY_TIMING(960, 8, 42, 13, 9, 9, 15, 11, 7, 12), },
> + { DSIM_DPHY_TIMING(950, 8, 41, 13, 9, 9, 14, 11, 7, 11), },
> + { DSIM_DPHY_TIMING(940, 8, 41, 13, 8, 9, 14, 11, 7, 11), },
> + { DSIM_DPHY_TIMING(930, 8, 40, 13, 8, 8, 14, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(920, 8, 40, 13, 8, 8, 14, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(910, 8, 39, 13, 8, 8, 14, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(900, 7, 39, 13, 8, 8, 13, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(890, 7, 38, 13, 8, 8, 13, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(880, 7, 38, 12, 8, 8, 13, 11, 6, 11), },
> + { DSIM_DPHY_TIMING(870, 7, 38, 12, 8, 8, 13, 11, 6, 10), },
> + { DSIM_DPHY_TIMING(860, 7, 37, 12, 8, 8, 13, 11, 6, 10), },
> + { DSIM_DPHY_TIMING(850, 7, 37, 12, 8, 8, 13, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(840, 7, 36, 12, 8, 8, 12, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(830, 7, 36, 12, 8, 8, 12, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(820, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(810, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(800, 7, 35, 12, 7, 7, 12, 10, 6, 10), },
> + { DSIM_DPHY_TIMING(790, 6, 34, 12, 7, 7, 11, 10, 5, 9), },
> + { DSIM_DPHY_TIMING(780, 6, 34, 12, 7, 7, 11, 10, 5, 9), },
> + { DSIM_DPHY_TIMING(770, 6, 33, 12, 7, 7, 11, 10, 5, 9), },
> + { DSIM_DPHY_TIMING(760, 6, 33, 12, 7, 7, 11, 10, 5, 9), },
> + { DSIM_DPHY_TIMING(750, 6, 32, 12, 7, 7, 11, 9, 5, 9), },
> + { DSIM_DPHY_TIMING(740, 6, 32, 11, 7, 7, 11, 9, 5, 9), },
> + { DSIM_DPHY_TIMING(730, 6, 31, 11, 7, 7, 10, 9, 5, 9), },
> + { DSIM_DPHY_TIMING(720, 6, 31, 11, 7, 6, 10, 9, 5, 9), },
> + { DSIM_DPHY_TIMING(710, 6, 31, 11, 6, 6, 10, 9, 5, 8), },
> + { DSIM_DPHY_TIMING(700, 6, 30, 11, 6, 6, 10, 9, 5, 8), },
> + { DSIM_DPHY_TIMING(690, 5, 30, 11, 6, 6, 10, 9, 5, 8), },
> + { DSIM_DPHY_TIMING(680, 5, 29, 11, 6, 6, 9, 9, 5, 8), },
> + { DSIM_DPHY_TIMING(670, 5, 29, 11, 6, 6, 9, 9, 5, 8), },
> + { DSIM_DPHY_TIMING(660, 5, 28, 11, 6, 6, 9, 9, 4, 8), },
> + { DSIM_DPHY_TIMING(650, 5, 28, 11, 6, 6, 9, 9, 4, 8), },
> + { DSIM_DPHY_TIMING(640, 5, 28, 11, 6, 6, 9, 8, 4, 8), },
> + { DSIM_DPHY_TIMING(630, 5, 27, 11, 6, 6, 9, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(620, 5, 27, 11, 6, 6, 8, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(610, 5, 26, 10, 6, 5, 8, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(600, 5, 26, 10, 6, 5, 8, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(590, 5, 25, 10, 5, 5, 8, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(580, 4, 25, 10, 5, 5, 8, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(570, 4, 24, 10, 5, 5, 7, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(560, 4, 24, 10, 5, 5, 7, 8, 4, 7), },
> + { DSIM_DPHY_TIMING(550, 4, 24, 10, 5, 5, 7, 8, 4, 6), },
> + { DSIM_DPHY_TIMING(540, 4, 23, 10, 5, 5, 7, 8, 4, 6), },
> + { DSIM_DPHY_TIMING(530, 4, 23, 10, 5, 5, 7, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(520, 4, 22, 10, 5, 5, 7, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(510, 4, 22, 10, 5, 5, 6, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(500, 4, 21, 10, 5, 4, 6, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(490, 4, 21, 10, 5, 4, 6, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(480, 4, 21, 9, 4, 4, 6, 7, 3, 6), },
> + { DSIM_DPHY_TIMING(470, 3, 20, 9, 4, 4, 6, 7, 3, 5), },
> + { DSIM_DPHY_TIMING(460, 3, 20, 9, 4, 4, 5, 7, 3, 5), },
> + { DSIM_DPHY_TIMING(450, 3, 19, 9, 4, 4, 5, 7, 3, 5), },
> + { DSIM_DPHY_TIMING(440, 3, 19, 9, 4, 4, 5, 7, 3, 5), },
> + { DSIM_DPHY_TIMING(430, 3, 18, 9, 4, 4, 5, 7, 3, 5), },
> + { DSIM_DPHY_TIMING(420, 3, 18, 9, 4, 4, 5, 6, 3, 5), },
> + { DSIM_DPHY_TIMING(410, 3, 17, 9, 4, 4, 5, 6, 3, 5), },
> + { DSIM_DPHY_TIMING(400, 3, 17, 9, 4, 3, 4, 6, 3, 5), },
> + { DSIM_DPHY_TIMING(390, 3, 17, 9, 4, 3, 4, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(380, 3, 16, 9, 4, 3, 4, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(370, 2, 16, 9, 3, 3, 4, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(360, 2, 15, 9, 3, 3, 4, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(350, 2, 15, 9, 3, 3, 3, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(340, 2, 14, 8, 3, 3, 3, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(330, 2, 14, 8, 3, 3, 3, 6, 2, 4), },
> + { DSIM_DPHY_TIMING(320, 2, 14, 8, 3, 3, 3, 5, 2, 4), },
> + { DSIM_DPHY_TIMING(310, 2, 13, 8, 3, 3, 3, 5, 2, 3), },
> + { DSIM_DPHY_TIMING(300, 2, 13, 8, 3, 3, 3, 5, 2, 3), },
> + { DSIM_DPHY_TIMING(290, 2, 12, 8, 3, 2, 2, 5, 2, 3), },
> + { DSIM_DPHY_TIMING(280, 2, 12, 8, 3, 2, 2, 5, 2, 3), },
> + { DSIM_DPHY_TIMING(270, 2, 11, 8, 3, 2, 2, 5, 2, 3), },
> + { DSIM_DPHY_TIMING(260, 1, 11, 8, 3, 2, 2, 5, 1, 3), },
> + { DSIM_DPHY_TIMING(250, 1, 10, 8, 2, 2, 2, 5, 1, 3), },
> + { DSIM_DPHY_TIMING(240, 1, 9, 8, 2, 2, 1, 5, 1, 3), },
> + { DSIM_DPHY_TIMING(230, 1, 8, 8, 2, 2, 1, 5, 1, 2), },
> + { DSIM_DPHY_TIMING(220, 1, 8, 8, 2, 2, 1, 5, 1, 2), },
> + { DSIM_DPHY_TIMING(210, 1, 7, 7, 2, 2, 1, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(200, 1, 7, 7, 2, 2, 1, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(190, 1, 7, 7, 2, 1, 1, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(180, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(170, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(160, 1, 6, 7, 2, 1, 0, 4, 1, 2), },
> + { DSIM_DPHY_TIMING(150, 0, 5, 7, 2, 1, 0, 4, 1, 1), },
> + { DSIM_DPHY_TIMING(140, 0, 5, 7, 1, 1, 0, 4, 1, 1), },
> + { DSIM_DPHY_TIMING(130, 0, 4, 7, 1, 1, 0, 4, 0, 1), },
> + { DSIM_DPHY_TIMING(120, 0, 4, 7, 1, 1, 0, 4, 0, 1), },
> + { DSIM_DPHY_TIMING(110, 0, 3, 7, 1, 0, 0, 4, 0, 1), },
> + { DSIM_DPHY_TIMING(100, 0, 3, 7, 1, 0, 0, 3, 0, 1), },
> + { DSIM_DPHY_TIMING(90, 0, 2, 7, 1, 0, 0, 3, 0, 1), },
> + { DSIM_DPHY_TIMING(80, 0, 2, 6, 1, 0, 0, 3, 0, 1), },
> +};
> +
> +#endif
> 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 {

2023-04-17 08:48:27

by Lucas Stach

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

Hi Adam,

Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> If there is more than one lane, the HFP, HBP, and HSA is calculated in
> bytes/pixel, then they are divided amongst the different lanes with some
> additional overhead. This is necessary to achieve higher resolutions while
> keeping the pixel clocks lower as the number of lanes increase.
>

In the testing I did to come up with my patch "drm: bridge: samsung-
dsim: fix blanking packet size calculation" the number of lanes didn't
make any difference. My testing might be flawed, as I could only
measure the blanking after translation from MIPI DSI to DPI, so I'm
interested to know what others did here. How did you validate the
blanking with your patch? Would you have a chance to test my patch and
see if it works or breaks in your setup?

Regards,
Lucas

> Signed-off-by: Adam Ford <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e0a402a85787..1ccbad4ea577 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -215,6 +215,7 @@
> #define DSI_RX_FIFO_SIZE 256
> #define DSI_XFER_TIMEOUT_MS 100
> #define DSI_RX_FIFO_EMPTY 0x30800002
> +#define DSI_HSYNC_PKT_OVERHEAD 6
>
> #define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
>
> @@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> | DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
> samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);
>
> - reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> - | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> - samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> + /*
> + * If there is more than one lane, the HFP, HBP, and HSA
> + * is calculated in bytes/pixel, then they are divided
> + * amongst the different lanes with some additional
> + * overhead correction
> + */
> + if (dsi->lanes > 1) {
> + u32 hfp, hbp, hsa;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format) / 8;
> +
> + hfp = ((m->hsync_start - m->hdisplay) * bpp) / dsi->lanes;
> + hfp -= (hfp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> +
> + hbp = ((m->htotal - m->hsync_end) * bpp) / dsi->lanes;
> + hbp -= (hbp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
>
> - reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> - | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> - samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> + hsa = ((m->hsync_end - m->hsync_start) * bpp) / dsi->lanes;
> + hsa -= (hsa > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> +
> + reg = DSIM_MAIN_HFP(hfp) | DSIM_MAIN_HBP(hbp);
> + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> +
> + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> + | DSIM_MAIN_HSA(hsa);
> + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> + } else {
> + reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> + | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> +
> + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> + | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> + }
> }
> reg = DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
> DSIM_MAIN_VRESOL(m->vdisplay, num_bits_resol);

2023-04-17 12:00:40

by Adam Ford

Subject: Re: [PATCH 4/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing

On Mon, Apr 17, 2023 at 3:38 AM Lucas Stach <[email protected]> wrote:
>
> Hi Adam,
>
> Am Samstag, dem 15.04.2023 um 05:41 -0500 schrieb Adam Ford:
> > NXP uses a lookup table to determine the various values for
> > the PHY Timing based on the clock rate in their downstream
> > kernel. Since the input clock can be variable, the phy
> > settings 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.
> >
>
> I haven't checked if this generates values close to the ones in this
> table, but I guess it should be worth a try to use
> phy_mipi_dphy_get_default_config() instead.

I didn't know that was a thing. I like that idea much better than the
table. I just pulled what NXP had and tweaked it to fit the mainline.
I'll give it a try in the next few days, when I have some more time.

adam
>
> Regards,
> Lucas
>
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 85 +++++++--
> > drivers/gpu/drm/bridge/samsung-dsim.h | 254 ++++++++++++++++++++++++++
> > include/drm/bridge/samsung-dsim.h | 1 +
> > 3 files changed, 326 insertions(+), 14 deletions(-)
> > create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.h
> >
<snip>
>

2023-04-17 12:08:57

by Adam Ford

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
>
> Hi Adam,
>
> Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > bytes/pixel, then they are divided amongst the different lanes with some
> > additional overhead. This is necessary to achieve higher resolutions while
> > keeping the pixel clocks lower as the number of lanes increase.
> >
>
> In the testing I did to come up with my patch "drm: bridge: samsung-
> dsim: fix blanking packet size calculation" the number of lanes didn't
> make any difference. My testing might be flawed, as I could only
> measure the blanking after translation from MIPI DSI to DPI, so I'm
> interested to know what others did here. How did you validate the
> blanking with your patch? Would you have a chance to test my patch and
> see if it works or breaks in your setup?

Mine was purely by trial and error. I don't have a scope, nor do I
have a copy of the MIPI DSI spec, so if the image sync'd with my
monitor, I treated it as successful.

I can give yours a try, but it might be a few days since I've only
been working on this stuff a bit in my spare time.

adam

>
> Regards,
> Lucas
>
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
> > 1 file changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index e0a402a85787..1ccbad4ea577 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -215,6 +215,7 @@
> > #define DSI_RX_FIFO_SIZE 256
> > #define DSI_XFER_TIMEOUT_MS 100
> > #define DSI_RX_FIFO_EMPTY 0x30800002
> > +#define DSI_HSYNC_PKT_OVERHEAD 6
> >
> > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
> >
> > @@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> > | DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
> > samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);
> >
> > - reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> > - | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> > - samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > + /*
> > + * If there is more than one lane, the HFP, HBP, and HSA
> > + * is calculated in bytes/pixel, then they are divided
> > + * amongst the different lanes with some additional
> > + * overhead correction
> > + */
> > + if (dsi->lanes > 1) {
> > + u32 hfp, hbp, hsa;
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format) / 8;
> > +
> > + hfp = ((m->hsync_start - m->hdisplay) * bpp) / dsi->lanes;
> > + hfp -= (hfp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> > +
> > + hbp = ((m->htotal - m->hsync_end) * bpp) / dsi->lanes;
> > + hbp -= (hbp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> >
> > - reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > - | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> > - samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > + hsa = ((m->hsync_end - m->hsync_start) * bpp) / dsi->lanes;
> > + hsa -= (hsa > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> > +
> > + reg = DSIM_MAIN_HFP(hfp) | DSIM_MAIN_HBP(hbp);
> > + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > +
> > + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > + | DSIM_MAIN_HSA(hsa);
> > + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > + } else {
> > + reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> > + | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> > + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > +
> > + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > + | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> > + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > + }
> > }
> > reg = DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
> > DSIM_MAIN_VRESOL(m->vdisplay, num_bits_resol);
>

2023-04-17 12:09:24

by Adam Ford

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On Sun, Apr 16, 2023 at 5:13 PM Marek Vasut <[email protected]> wrote:
>
> On 4/15/23 12:41, Adam Ford wrote:
> > The high-speed clock is hard-coded to the burst-clock
> > frequency specified in the device tree. However, when
> > using devices like certain bridge chips without burst mode
> > and varying resolutions and refresh rates, it may be
> > necessary to set the high-speed clock dynamically based
> > on the desired pixel clock for the connected device.
>
> The link rate negotiation should happen internally between the nearest
> bridge and DSIM, so please add that to DRM core instead of hacking
> around it by tweaking the HS clock again.

I thought you tried to add something like this before and had some resistance.

The Pixel clock is set by the bridge already without any new code
added to the DRM core.. I am just reading that value that's there,
and setting the clock accordingly. I don't see how this is a hack.

adam

2023-04-17 20:16:08

by Marek Vasut

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On 4/17/23 13:57, Adam Ford wrote:
> On Sun, Apr 16, 2023 at 5:13 PM Marek Vasut <[email protected]> wrote:
>>
>> On 4/15/23 12:41, Adam Ford wrote:
>>> The high-speed clock is hard-coded to the burst-clock
>>> frequency specified in the device tree. However, when
>>> using devices like certain bridge chips without burst mode
>>> and varying resolutions and refresh rates, it may be
>>> necessary to set the high-speed clock dynamically based
>>> on the desired pixel clock for the connected device.
>>
>> The link rate negotiation should happen internally between the nearest
>> bridge and DSIM, so please add that to DRM core instead of hacking
>> around it by tweaking the HS clock again.
>
> I thought you tried to add something like this before and had some resistance.

Yes, all my attempts were rejected by a single reviewer. I suspended my
efforts in that area for now.

> The Pixel clock is set by the bridge already without any new code
> added to the DRM core.. I am just reading that value that's there,
> and setting the clock accordingly. I don't see how this is a hack.

Assume you have a DSI-to-HDMI bridge attached to your DSIM bridge, it
operates in non-burst mode, like ADV7533 . How would you configure the
HS clock rate for such a bridge in DT ? (hint: you cannot, because the
required clock comes from the EDID, which may not be available just yet)

2023-04-17 22:42:18

by Adam Ford

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On Mon, Apr 17, 2023 at 3:08 PM Marek Vasut <[email protected]> wrote:
>
> On 4/17/23 13:57, Adam Ford wrote:
> > On Sun, Apr 16, 2023 at 5:13 PM Marek Vasut <[email protected]> wrote:
> >>
> >> On 4/15/23 12:41, Adam Ford wrote:
> >>> The high-speed clock is hard-coded to the burst-clock
> >>> frequency specified in the device tree. However, when
> >>> using devices like certain bridge chips without burst mode
> >>> and varying resolutions and refresh rates, it may be
> >>> necessary to set the high-speed clock dynamically based
> >>> on the desired pixel clock for the connected device.
> >>
> >> The link rate negotiation should happen internally between the nearest
> >> bridge and DSIM, so please add that to DRM core instead of hacking
> >> around it by tweaking the HS clock again.
> >
> > I thought you tried to add something like this before and had some resistance.
>
> Yes, all my attempts were rejected by a single reviewer. I suspended my
> efforts in that area for now.
>
> > The Pixel clock is set by the bridge already without any new code
> > added to the DRM core.. I am just reading that value that's there,
> > and setting the clock accordingly. I don't see how this is a hack.
>
> Assume you have a DSI-to-HDMI bridge attached to your DSIM bridge, it
> operates in non-burst mode, like ADV7533 . How would you configure the

I have an ADV7535

> HS clock rate for such a bridge in DT ? (hint: you cannot, because the
> required clock comes from the EDID, which may not be available just yet)

The whole idea is that you wouldn't want to or need to configure the
clock speed in the device tree because it comes from the
EDID->bridge->DSI.

I've tested this configuration on imx8mm, imx8mn, and imx8mp and I can
change the resolution and refresh rate on the fly and the DSI will
automatically readjust accordingly. If you fixed the clock in the
device tree, you wouldn't be able to do that, and that was the point
of this patch.


adam

2023-04-17 23:31:53

by Marek Vasut

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On 4/18/23 00:24, Adam Ford wrote:
> On Mon, Apr 17, 2023 at 3:08 PM Marek Vasut <[email protected]> wrote:
>>
>> On 4/17/23 13:57, Adam Ford wrote:
>>> On Sun, Apr 16, 2023 at 5:13 PM Marek Vasut <[email protected]> wrote:
>>>>
>>>> On 4/15/23 12:41, Adam Ford wrote:
>>>>> The high-speed clock is hard-coded to the burst-clock
>>>>> frequency specified in the device tree. However, when
>>>>> using devices like certain bridge chips without burst mode
>>>>> and varying resolutions and refresh rates, it may be
>>>>> necessary to set the high-speed clock dynamically based
>>>>> on the desired pixel clock for the connected device.
>>>>
>>>> The link rate negotiation should happen internally between the nearest
>>>> bridge and DSIM, so please add that to DRM core instead of hacking
>>>> around it by tweaking the HS clock again.
>>>
>>> I thought you tried to add something like this before and had some resistance.
>>
>> Yes, all my attempts were rejected by a single reviewer. I suspended my
>> efforts in that area for now.
>>
>>> The Pixel clock is set by the bridge already without any new code
>>> added to the DRM core.. I am just reading that value that's there,
>>> and setting the clock accordingly. I don't see how this is a hack.
>>
>> Assume you have a DSI-to-HDMI bridge attached to your DSIM bridge, it
>> operates in non-burst mode, like ADV7533 . How would you configure the
>
> I have an ADV7535
>
>> HS clock rate for such a bridge in DT ? (hint: you cannot, because the
>> required clock comes from the EDID, which may not be available just yet)
>
> The whole idea is that you wouldn't want to or need to configure the
> clock speed in the device tree because it comes from the
> EDID->bridge->DSI.
>
> I've tested this configuration on imx8mm, imx8mn, and imx8mp and I can
> change the resolution and refresh rate on the fly and the DSI will
> automatically readjust accordingly. If you fixed the clock in the
> device tree, you wouldn't be able to do that, and that was the point
> of this patch.

Uh, I retract my comment, I was clearly confused here and we're talking
about the same thing.

2023-04-18 02:45:10

by Adam Ford

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On Sun, Apr 16, 2023 at 5:08 PM Marek Vasut <[email protected]> wrote:
>
> On 4/15/23 12:41, Adam Ford wrote:
> > Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> > having an entry in the device tree for samsung,pll-clock-frequency.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 9fec32b44e05..73f0c3fbbdf5 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> > struct device_node *node = dev->of_node;
> > int ret;
> >
> > - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> > - &dsi->pll_clk_rate);
> > - if (ret < 0)
> > - return ret;
> > -
> > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> > &dsi->burst_clk_rate);
> > if (ret < 0)
>
> Does this break compatibility with old samsung DTs ?

My goal here was to declutter the device tree stuff and fetch data
automatically if possible. What if I changed this to make them
optional? If they exist, we can use them, if they don't exist, we
could read the clock rate. Would that be acceptable?

adam

2023-04-18 02:55:56

by Adam Ford

Subject: Re: [PATCH 4/6] drm: bridge: samsung-dsim: Dynamically configure DPHY timing

On Mon, Apr 17, 2023 at 6:53 AM Adam Ford <[email protected]> wrote:
>
> On Mon, Apr 17, 2023 at 3:38 AM Lucas Stach <[email protected]> wrote:
> >
> > Hi Adam,
> >
> > Am Samstag, dem 15.04.2023 um 05:41 -0500 schrieb Adam Ford:
> > > NXP uses a lookup table to determine the various values for
> > > the PHY Timing based on the clock rate in their downstream
> > > kernel. Since the input clock can be variable, the phy
> > > settings 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.
> > >
> >
> > I haven't checked if this generates values close to the ones in this
> > table, but I guess it should be worth a try to use
> > phy_mipi_dphy_get_default_config() instead.
>
> I didn't know that was a thing. I like that idea much better than the
> table. I just pulled what NXP had and tweaked it to fit the mainline.
> I'll give it a try in the next few days, when I have some more time.

I tried phy_mipi_dphy_get_default_config() and the function return
different values than what NXP's table returns, and the screen doesn't
sync properly. I will try to figure out the mathematical calculation
to generate the values for this DSIM instead of using a lookup table.

adam
>
> adam
> >
> > Regards,
> > Lucas
> >
> > > Signed-off-by: Adam Ford <[email protected]>
> > > ---
> > > drivers/gpu/drm/bridge/samsung-dsim.c | 85 +++++++--
> > > drivers/gpu/drm/bridge/samsung-dsim.h | 254 ++++++++++++++++++++++++++
> > > include/drm/bridge/samsung-dsim.h | 1 +
> > > 3 files changed, 326 insertions(+), 14 deletions(-)
> > > create mode 100644 drivers/gpu/drm/bridge/samsung-dsim.h
> > >
> <snip>
> >

2023-04-18 08:35:10

by Marek Vasut

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On 4/18/23 04:29, Adam Ford wrote:
> On Sun, Apr 16, 2023 at 5:08 PM Marek Vasut <[email protected]> wrote:
>>
>> On 4/15/23 12:41, Adam Ford wrote:
>>> Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
>>> having an entry in the device tree for samsung,pll-clock-frequency.
>>>
>>> Signed-off-by: Adam Ford <[email protected]>
>>> ---
>>> drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
>>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> index 9fec32b44e05..73f0c3fbbdf5 100644
>>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>>> @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
>>> struct device_node *node = dev->of_node;
>>> int ret;
>>>
>>> - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
>>> - &dsi->pll_clk_rate);
>>> - if (ret < 0)
>>> - return ret;
>>> -
>>> ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
>>> &dsi->burst_clk_rate);
>>> if (ret < 0)
>>
>> Does this break compatibility with old samsung DTs ?
>
> My goal here was to declutter the device tree stuff and fetch data
> automatically if possible. What if I changed this to make them
> optional? If they exist, we can use them, if they don't exist, we
> could read the clock rate. Would that be acceptable?

If you do not see any potential problem with ignoring the DT property
altogether, that would be better of course, but I think you cannot do
that with old DTs, so you should retain backward compatibility fallback,
yes. What do you think ?

2023-04-18 08:53:08

by Lucas Stach

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

Am Dienstag, dem 18.04.2023 um 10:30 +0200 schrieb Marek Vasut:
> On 4/18/23 04:29, Adam Ford wrote:
> > On Sun, Apr 16, 2023 at 5:08 PM Marek Vasut <[email protected]> wrote:
> > >
> > > On 4/15/23 12:41, Adam Ford wrote:
> > > > Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> > > > having an entry in the device tree for samsung,pll-clock-frequency.
> > > >
> > > > Signed-off-by: Adam Ford <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> > > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > index 9fec32b44e05..73f0c3fbbdf5 100644
> > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> > > > struct device_node *node = dev->of_node;
> > > > int ret;
> > > >
> > > > - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> > > > - &dsi->pll_clk_rate);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > -
> > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> > > > &dsi->burst_clk_rate);
> > > > if (ret < 0)
> > >
> > > Does this break compatibility with old samsung DTs ?
> >
> > My goal here was to declutter the device tree stuff and fetch data
> > automatically if possible. What if I changed this to make them
> > optional? If they exist, we can use them, if they don't exist, we
> > could read the clock rate. Would that be acceptable?
>
> If you do not see any potential problem with ignoring the DT property
> altogether, that would be better of course, but I think you cannot do
> that with old DTs, so you should retain backward compatibility fallback,
> yes. What do you think ?

I'm very much in favor of this patch, but I also think we shouldn't
risk breaking Samsung devices, where we don't now 100% that the input
clock rate provided by the clock driver is correct.

So I think the right approach is to use "samsung,pll-clock-frequency"
when present in DT and get it from the clock provider otherwise. Then
just remove the property from the DTs where we can validate that the
input clock rate is correct, i.e. all i.MX8M*.

Regards,
Lucas

2023-04-18 11:58:45

by Adam Ford

Subject: Re: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

On Mon, Apr 17, 2023 at 2:00 AM Alexander Stein
<[email protected]> wrote:
>
> Hi,
>
> Am Montag, 17. April 2023, 00:31:24 CEST schrieb Adam Ford:
> > On Sun, Apr 16, 2023 at 5:07 PM Marek Vasut <[email protected]> wrote:
> > > On 4/15/23 12:40, Adam Ford wrote:
> > > > According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
> > > > and max values for M and the frequency range for the VCO_out
> > > > calculator were incorrect. This also appears to be the case for the
> > > > imx8mn and imx8mp.
> > > >
> > > > To fix this, make new variables to hold the min and max values of m
> > > > and the minimum value of VCO_out, and update the PMS calculator to
> > > > use these new variables instead of using hard-coded values to keep
> > > > the backwards compatibility with other parts using this driver.
> > >
> > > [...]
> > >
> > > > static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data =
> > > > {
> > > >
> > > > @@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data
> > > > imx8mm_dsi_driver_data = {> >
> > > > */
> > > >
> > > > .pll_p_offset = 14,
> > > > .reg_values = imx8mm_dsim_reg_values,
> > > >
> > > > + .m_min = 64,
> > > > + .m_max = 1023,
> > > > + .vco_min = 1050,
> > >
> > > You might want to call this 'min_freq' since there is a 'max_freq' which
> > > seems to indicate what VCO max frequency is.
> > >
> > > Note that the same datasheet contains the following information:
> > > "
> > > MIPI_DPHY_M_PLLPMS field descriptions
> > >
> > > 12–4 PMS_M
> > > Specifies the PLL PMS value for the M divider
> > > NOTE: The programmable divider range should be within 25 to 125 to
> > > ensure PLL stability.
> >
> > I was confused by this because this statement is not consistent with
> > the link they reference jumps me to the table where it reads M is
> > between 64 and 1023.
> >
> > > NOTE: The M and P divider values should be considered together to ensure
> > > VCO ouput frequency
> > > (VCO_out) range is between 350 MHz to 750 MHz.
> > > Please refer to the topic DPHY PLL for more information.
> >
> > I was confused by this too, because the NXP documentation reads the
> > 350 - 750MHz that you state, but "Table 13-45: DPHY PLL Parameters"
> > which immediately follows that sentence on page 4158 shows VCO_out is
> > between 1050-2100 MHz.
> >
> > I compared the PMS values for a variety of frequencies to those that
> > were set in the downstream NXP code, and the PMS values matched.
> > Maybe someone from NXP can explain the discrepancy.
>
> Also note that, according to Table 13-28 in RM (Rev 2 07/2022) for i.MX8M
> Nano, VCO_out and Fout has a maximum of 1500MHz only. Although the note above
> mentions a range of 1050-2100MHz...

I looked up the limits in NXP's downstream kernel [1] , and I believe
these values match the table in the reference manual instead of the
conflicting sentence. I am guessing this is why the PMS values I get
match those of the NXP downstream kernel.

adam

[1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/imx/sec_mipi_pll_1432x.h#L38

>
> Best regards,
> Alexander
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>

2023-04-19 10:47:17

by Adam Ford

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On Tue, Apr 18, 2023 at 3:47 AM Lucas Stach <[email protected]> wrote:
>
> Am Dienstag, dem 18.04.2023 um 10:30 +0200 schrieb Marek Vasut:
> > On 4/18/23 04:29, Adam Ford wrote:
> > > On Sun, Apr 16, 2023 at 5:08 PM Marek Vasut <[email protected]> wrote:
> > > >
> > > > On 4/15/23 12:41, Adam Ford wrote:
> > > > > Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> > > > > having an entry in the device tree for samsung,pll-clock-frequency.
> > > > >
> > > > > Signed-off-by: Adam Ford <[email protected]>
> > > > > ---
> > > > > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> > > > > 1 file changed, 6 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > > index 9fec32b44e05..73f0c3fbbdf5 100644
> > > > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > > > @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> > > > > struct device_node *node = dev->of_node;
> > > > > int ret;
> > > > >
> > > > > - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> > > > > - &dsi->pll_clk_rate);
> > > > > - if (ret < 0)
> > > > > - return ret;
> > > > > -
> > > > > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> > > > > &dsi->burst_clk_rate);
> > > > > if (ret < 0)
> > > >
> > > > Does this break compatibility with old samsung DTs ?
> > >
> > > My goal here was to declutter the device tree stuff and fetch data
> > > automatically if possible. What if I changed this to make them
> > > optional? If they exist, we can use them, if they don't exist, we
> > > could read the clock rate. Would that be acceptable?
> >
> > If you do not see any potential problem with ignoring the DT property
> > altogether, that would be better of course, but I think you cannot do
> > that with old DTs, so you should retain backward compatibility fallback,
> > yes. What do you think ?
>
> I'm very much in favor of this patch, but I also think we shouldn't
> risk breaking Samsung devices, where we don't now 100% that the input
> clock rate provided by the clock driver is correct.
>
> So I think the right approach is to use "samsung,pll-clock-frequency"
> when present in DT and get it from the clock provider otherwise. Then
> just remove the property from the DTs where we can validate that the
> input clock rate is correct, i.e. all i.MX8M*.

I'll update this accordingly when I do a V2 of this series.

adam
>
> Regards,
> Lucas

2023-04-19 10:53:29

by Adam Ford

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
>
> On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> >
> > Hi Adam,
> >
> > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > bytes/pixel, then they are divided amongst the different lanes with some
> > > additional overhead. This is necessary to achieve higher resolutions while
> > > keeping the pixel clocks lower as the number of lanes increase.
> > >
> >
> > In the testing I did to come up with my patch "drm: bridge: samsung-
> > dsim: fix blanking packet size calculation" the number of lanes didn't
> > make any difference. My testing might be flawed, as I could only
> > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > interested to know what others did here. How did you validate the
> > blanking with your patch? Would you have a chance to test my patch and
> > see if it works or breaks in your setup?

Lucas,

I tried your patch instead of mine. Yours is dependent on the
hs_clock being always set to the burst clock which is configured by
the device tree. I unrolled a bit of my stuff and replaced it with
yours. It worked at 1080p, but when I tried a few other resolutions,
they did not work. I assume it's because the DSI clock is fixed and
not changing based on the pixel clock. In the version I did, I only
did that math when the lanes were > 1. In your patch, you divide by 8,
and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
that just in case the bpp ever changes from 8. Overall, I think our
patches basically do the same thing.

adam

>
> Mine was purely by trial and error. I don't have a scope, nor do I
> have a copy of the MIPI DSI spec, so if the image sync'd with my
> monitor, I treated it as successful.
>
> I can give yours a try, but it might be a few days since I've only
> been working on this stuff a bit in my spare time.
>
> adam
>
> >
> > Regards,
> > Lucas
> >
> > > Signed-off-by: Adam Ford <[email protected]>
> > > ---
> > > drivers/gpu/drm/bridge/samsung-dsim.c | 40 +++++++++++++++++++++++----
> > > 1 file changed, 34 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > index e0a402a85787..1ccbad4ea577 100644
> > > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > > @@ -215,6 +215,7 @@
> > > #define DSI_RX_FIFO_SIZE 256
> > > #define DSI_XFER_TIMEOUT_MS 100
> > > #define DSI_RX_FIFO_EMPTY 0x30800002
> > > +#define DSI_HSYNC_PKT_OVERHEAD 6
> > >
> > > #define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
> > >
> > > @@ -879,13 +880,40 @@ static void samsung_dsim_set_display_mode(struct samsung_dsim *dsi)
> > > | DSIM_MAIN_VBP(m->vtotal - m->vsync_end);
> > > samsung_dsim_write(dsi, DSIM_MVPORCH_REG, reg);
> > >
> > > - reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> > > - | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> > > - samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > > + /*
> > > + * If there is more than one lane, the HFP, HBP, and HSA
> > > + * is calculated in bytes/pixel, then they are divided
> > > + * amongst the different lanes with some additional
> > > + * overhead correction
> > > + */
> > > + if (dsi->lanes > 1) {
> > > + u32 hfp, hbp, hsa;
> > > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format) / 8;
> > > +
> > > + hfp = ((m->hsync_start - m->hdisplay) * bpp) / dsi->lanes;
> > > + hfp -= (hfp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> > > +
> > > + hbp = ((m->htotal - m->hsync_end) * bpp) / dsi->lanes;
> > > + hbp -= (hbp > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> > >
> > > - reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > > - | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> > > - samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > > + hsa = ((m->hsync_end - m->hsync_start) * bpp) / dsi->lanes;
> > > + hsa -= (hsa > DSI_HSYNC_PKT_OVERHEAD) ? DSI_HSYNC_PKT_OVERHEAD : 0;
> > > +
> > > + reg = DSIM_MAIN_HFP(hfp) | DSIM_MAIN_HBP(hbp);
> > > + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > > +
> > > + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > > + | DSIM_MAIN_HSA(hsa);
> > > + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > > + } else {
> > > + reg = DSIM_MAIN_HFP(m->hsync_start - m->hdisplay)
> > > + | DSIM_MAIN_HBP(m->htotal - m->hsync_end);
> > > + samsung_dsim_write(dsi, DSIM_MHPORCH_REG, reg);
> > > +
> > > + reg = DSIM_MAIN_VSA(m->vsync_end - m->vsync_start)
> > > + | DSIM_MAIN_HSA(m->hsync_end - m->hsync_start);
> > > + samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
> > > + }
> > > }
> > > reg = DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
> > > DSIM_MAIN_VRESOL(m->vdisplay, num_bits_resol);
> >

2023-04-20 02:41:25

by Adam Ford

Subject: Re: [PATCH 5/6] drm: bridge: samsung-dsim: Support non-burst mode

On Mon, Apr 17, 2023 at 6:23 PM Marek Vasut <[email protected]> wrote:
>
> On 4/18/23 00:24, Adam Ford wrote:
> > On Mon, Apr 17, 2023 at 3:08 PM Marek Vasut <[email protected]> wrote:
> >>
> >> On 4/17/23 13:57, Adam Ford wrote:
> >>> On Sun, Apr 16, 2023 at 5:13 PM Marek Vasut <[email protected]> wrote:
> >>>>
> >>>> On 4/15/23 12:41, Adam Ford wrote:
> >>>>> The high-speed clock is hard-coded to the burst-clock
> >>>>> frequency specified in the device tree. However, when
> >>>>> using devices like certain bridge chips without burst mode
> >>>>> and varying resolutions and refresh rates, it may be
> >>>>> necessary to set the high-speed clock dynamically based
> >>>>> on the desired pixel clock for the connected device.
> >>>>
> >>>> The link rate negotiation should happen internally between the nearest
> >>>> bridge and DSIM, so please add that to DRM core instead of hacking
> >>>> around it by tweaking the HS clock again.
> >>>
> >>> I thought you tried to add something like this before and had some resistance.
> >>
> >> Yes, all my attempts were rejected by a single reviewer. I suspended my
> >> efforts in that area for now.
> >>
> >>> The Pixel clock is set by the bridge already without any new code
> >>> added to the DRM core.. I am just reading that value that's there,
> >>> and setting the clock accordingly. I don't see how this is a hack.
> >>
> >> Assume you have a DSI-to-HDMI bridge attached to your DSIM bridge, it
> >> operates in non-burst mode, like ADV7533 . How would you configure the
> >
> > I have an ADV7535
> >
> >> HS clock rate for such a bridge in DT ? (hint: you cannot, because the
> >> required clock comes from the EDID, which may not be available just yet)
> >
> > The whole idea is that you wouldn't want to or need to configure the
> > clock speed in the device tree because it comes from the
> > EDID->bridge->DSI.
> >
> > I've tested this configuration on imx8mm, imx8mn, and imx8mp and I can
> > change the resolution and refresh rate on the fly and the DSI will
> > automatically readjust accordingly. If you fixed the clock in the
> > device tree, you wouldn't be able to do that, and that was the point
> > of this patch.
>
> Uh, I retract my comment, I was clearly confused here and we're talking
> about the same thing.

I'm working on a V2 for this series. Are you OK with this if I update
the commit message a bit to make it more clear?

adam

2023-04-20 13:07:53

by Lucas Stach

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

Hi Adam,

Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> >
> > On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> > >
> > > Hi Adam,
> > >
> > > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > > bytes/pixel, then they are divided amongst the different lanes with some
> > > > additional overhead. This is necessary to achieve higher resolutions while
> > > > keeping the pixel clocks lower as the number of lanes increase.
> > > >
> > >
> > > In the testing I did to come up with my patch "drm: bridge: samsung-
> > > dsim: fix blanking packet size calculation" the number of lanes didn't
> > > make any difference. My testing might be flawed, as I could only
> > > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > > interested to know what others did here. How did you validate the
> > > blanking with your patch? Would you have a chance to test my patch and
> > > see if it works or breaks in your setup?
>
> Lucas,
>
> I tried your patch instead of mine. Yours is dependent on the
> hs_clock being always set to the burst clock which is configured by
> the device tree. I unrolled a bit of my stuff and replaced it with
> yours. It worked at 1080p, but when I tried a few other resolutions,
> they did not work. I assume it's because the DSI clock is fixed and
> not changing based on the pixel clock. In the version I did, I only
> did that math when the lanes were > 1. In your patch, you divide by 8,
> and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> that just in case the bpp ever changes from 8. Overall, I think our
> patches basically do the same thing.

The calculations in your and my patch are quite different. I'm not
taking into account the number of lanes or the MIPI format. I'm basing
the blanking size purely on the ratio between MIPI DSI byte clock and
the DPI interface clock. It's quite counter-intuitive that the host
would scale the blanking to the number of lanes automatically, but
still require the MIPI packet offset removed, but that's what my
measurements showed to produce the correct blanking after translation
to DPI by the TC358767 bridge chip.

If you dynamically scale the HS clock, then you would need to input the
real used HS clock to the calculation in my patch, instead of the fixed
burst mode rate.

Regards,
Lucas

2023-04-20 13:29:02

by Adam Ford

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
>
> Hi Adam,
>
> Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> > On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> > >
> > > On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> > > >
> > > > Hi Adam,
> > > >
> > > > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > > > bytes/pixel, then they are divided amongst the different lanes with some
> > > > > additional overhead. This is necessary to achieve higher resolutions while
> > > > > keeping the pixel clocks lower as the number of lanes increase.
> > > > >
> > > >
> > > > In the testing I did to come up with my patch "drm: bridge: samsung-
> > > > dsim: fix blanking packet size calculation" the number of lanes didn't
> > > > make any difference. My testing might be flawed, as I could only
> > > > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > > > interested to know what others did here. How did you validate the
> > > > blanking with your patch? Would you have a chance to test my patch and
> > > > see if it works or breaks in your setup?
> >
> > Lucas,
> >
> > I tried your patch instead of mine. Yours is dependent on the
> > hs_clock being always set to the burst clock which is configured by
> > the device tree. I unrolled a bit of my stuff and replaced it with
> > yours. It worked at 1080p, but when I tried a few other resolutions,
> > they did not work. I assume it's because the DSI clock is fixed and
> > not changing based on the pixel clock. In the version I did, I only
> > did that math when the lanes were > 1. In your patch, you divide by 8,
> > and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> > that just in case the bpp ever changes from 8. Overall, I think our
> > patches basically do the same thing.
>
> The calculations in your and my patch are quite different. I'm not
> taking into account the number of lanes or the MIPI format. I'm basing

I was looking more at the division by 8 and the overhead correction of 6.
This number 6 also appears in the NXP downstream kernel [1]. I know
Marek V had some concerns about that.

> the blanking size purely on the ratio between MIPI DSI byte clock and
> the DPI interface clock. It's quite counter-intuitive that the host
> would scale the blanking to the number of lanes automatically, but
> still require the MIPI packet offset removed, but that's what my
> measurements showed to produce the correct blanking after translation
> to DPI by the TC358767 bridge chip.

How many lanes is your DSI interface using?

>
> If you dynamically scale the HS clock, then you would need to input the
> real used HS clock to the calculation in my patch, instead of the fixed
> burst mode rate.

I think what you're saying makes sense.

The code I originally modeled this from was from the NXP downstream
kernel where they define the calculation as being in words [2]. I am
not saying the NXP code is perfect, but the NXP code works. With this
series, my monitors are able to sync a bunch of different resolutions
from 1080p down to 640x480 and a bunch in between with various refresh
rates too. That was the goal of this series.

Instead of just using your patch as-is, I will adapt yours to use the
scaled clock to see how it behaves and get back to you. I have
finished reworking the dynamic DPHY settings, and I've fixed up making
the PLL device tree optional. If your patch works, I'll drop my
calculation and just build off what you have to use the scaled HS
clock when I submit the V2 and I'll make sure I CC you.

adam

[1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L270
[2] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L914

>
> Regards,
> Lucas

2023-04-20 13:54:50

by Lucas Stach

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

Am Donnerstag, dem 20.04.2023 um 08:24 -0500 schrieb Adam Ford:
> On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
> >
> > Hi Adam,
> >
> > Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> > > On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> > > >
> > > > On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> > > > >
> > > > > Hi Adam,
> > > > >
> > > > > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > > > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > > > > bytes/pixel, then they are divided amongst the different lanes with some
> > > > > > additional overhead. This is necessary to achieve higher resolutions while
> > > > > > keeping the pixel clocks lower as the number of lanes increase.
> > > > > >
> > > > >
> > > > > In the testing I did to come up with my patch "drm: bridge: samsung-
> > > > > dsim: fix blanking packet size calculation" the number of lanes didn't
> > > > > make any difference. My testing might be flawed, as I could only
> > > > > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > > > > interested to know what others did here. How did you validate the
> > > > > blanking with your patch? Would you have a chance to test my patch and
> > > > > see if it works or breaks in your setup?
> > >
> > > Lucas,
> > >
> > > I tried your patch instead of mine. Yours is dependent on the
> > > hs_clock being always set to the burst clock which is configured by
> > > the device tree. I unrolled a bit of my stuff and replaced it with
> > > yours. It worked at 1080p, but when I tried a few other resolutions,
> > > they did not work. I assume it's because the DSI clock is fixed and
> > > not changing based on the pixel clock. In the version I did, I only
> > > did that math when the lanes were > 1. In your patch, you divide by 8,
> > > and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> > > that just in case the bpp ever changes from 8. Overall, I think our
> > > patches basically do the same thing.
> >
> > The calculations in your and my patch are quite different. I'm not
> > taking into account the number of lanes or the MIPI format. I'm basing
>
> I was looking more at the division by 8 and the overhead correction of 6.
> This number 6 also appears in the NXP downstream kernel [1]. I know
> Marek V had some concerns about that.
>
Yea, I don't fully remember the details about the overhead. Need to
page that back in. The division by 8 in my patch is just to get from
the bit to a byte clock, nothing to do with the MIPI format bits per
channel or something like that.

> > the blanking size purely on the ratio between MIPI DSI byte clock and
> > the DPI interface clock. It's quite counter-intuitive that the host
> > would scale the blanking to the number of lanes automatically, but
> > still require the MIPI packet offset removed, but that's what my
> > measurements showed to produce the correct blanking after translation
> > to DPI by the TC358767 bridge chip.
>
> How many lanes is your DSI interface using?
>
When I did the measurements to come up with the patch, I varied the
number of lanes between 1 and 4. Different number of lanes didn't make
a difference. In fact trying to compensate for the number of lanes when
calculating the blanking size to program into the controller lead to
wildly wrong blanking on the DPI side of the external bridge.

> >
> > If you dynamically scale the HS clock, then you would need to input the
> > real used HS clock to the calculation in my patch, instead of the fixed
> > burst mode rate.
>
> I think what you're saying makes sense.
>
> The code I originally modeled this from was from the NXP downstream
> kernel where they define the calculation as being in words [2]. I am
> not saying the NXP code is perfect, but the NXP code works. With this
> series, my monitors are able to sync a bunch of different resolutions
> from 1080p down to 640x480 and a bunch in between with various refresh
> rates too. That was the goal of this series.
>
> Instead of just using your patch as-is, I will adapt yours to use the
> scaled clock to see how it behaves and get back to you.
>

Thanks, that would be very much appreciated.

I also don't assert that my calculation is perfect, as I also don't
have any more information and needed to resort to observing the
blanking after translation by the external bridge, so I hope we could
get some better understanding of how things really work by checking
what works for both our systems.

> I have
> finished reworking the dynamic DPHY settings, and I've fixed up making
> the PLL device tree optional. If your patch works, I'll drop my
> calculation and just build off what you have to use the scaled HS
> clock when I submit the V2 and I'll make sure I CC you.
>
Thanks, I'm open to re-do my measurements with your new patches.

Regards,
Lucas

> adam
>
> [1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L270
> [2] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L914
>
> >
> > Regards,
> > Lucas

2023-04-20 19:25:20

by Adam Ford

Subject: Re: [PATCH 2/6] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]

On Tue, Apr 18, 2023 at 6:53 AM Adam Ford <[email protected]> wrote:
>
> On Mon, Apr 17, 2023 at 2:00 AM Alexander Stein
> <[email protected]> wrote:
> >
> > Hi,
> >
> > Am Montag, 17. April 2023, 00:31:24 CEST schrieb Adam Ford:
> > > On Sun, Apr 16, 2023 at 5:07 PM Marek Vasut <[email protected]> wrote:
> > > > On 4/15/23 12:40, Adam Ford wrote:
> > > > > According to Table 13-45 of the i.MX8M Mini Reference Manual, the min
> > > > > and max values for M and the frequency range for the VCO_out
> > > > > calculator were incorrect. This also appears to be the case for the
> > > > > imx8mn and imx8mp.
> > > > >
> > > > > To fix this, make new variables to hold the min and max values of m
> > > > > and the minimum value of VCO_out, and update the PMS calculator to
> > > > > use these new variables instead of using hard-coded values to keep
> > > > > the backwards compatibility with other parts using this driver.
> > > >
> > > > [...]
> > > >
> > > > > static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data =
> > > > > {
> > > > >
> > > > > @@ -470,6 +485,9 @@ static const struct samsung_dsim_driver_data
> > > > > imx8mm_dsi_driver_data = {> >
> > > > > */
> > > > >
> > > > > .pll_p_offset = 14,
> > > > > .reg_values = imx8mm_dsim_reg_values,
> > > > >
> > > > > + .m_min = 64,
> > > > > + .m_max = 1023,
> > > > > + .vco_min = 1050,
> > > >
> > > > You might want to call this 'min_freq' since there is a 'max_freq' which
> > > > seems to indicate what VCO max frequency is.
> > > >
> > > > Note that the same datasheet contains the following information:
> > > > "
> > > > MIPI_DPHY_M_PLLPMS field descriptions
> > > >
> > > > 12–4 PMS_M
> > > > Specifies the PLL PMS value for the M divider
> > > > NOTE: The programmable divider range should be within 25 to 125 to
> > > > ensure PLL stability.
> > >
> > > I was confused by this because this statement is not consistent with
> > > the link they reference jumps me to the table where it reads M is
> > > between 64 and 1023.
> > >
> > > > NOTE: The M and P divider values should be considered together to ensure
> > > > VCO ouput frequency
> > > > (VCO_out) range is between 350 MHz to 750 MHz.
> > > > Please refer to the topic DPHY PLL for more information.
> > >
> > > I was confused by this too, because the NXP documentation reads the
> > > 350 - 750MHz that you state, but "Table 13-45: DPHY PLL Parameters"
> > > which immediately follows that sentence on page 4158 shows VCO_out is
> > > between 1050-2100 MHz.

I reached out to my NXP rep asking if the VCO_out is 350-750 or if it
should be 1050-2100, and received the following statement:

" Yes it is definitely wrong, the one that is part of the NOTE in
MIPI_DPHY_M_PLLPMS register table against PMS_P, PMS_M and PMS_S is
not correct. I will report this to Doc team, the one customer should
be take into account is the Table 13-40 DPHY PLL Parameters and the
Note above."

Table 13-40 (for the Nano) reads the VCO_out is 1050-2100, despite
other text stating 350-750MHz, so I believe this patch is appropriate.
I'll add the above statement to the commit message as confirmation
when I submit a V2 of this series.

adam
> > >
> > > I compared the PMS values for a variety of frequencies to those that
> > > were set in the downstream NXP code, and the PMS values matched.
> > > Maybe someone from NXP can explain the discrepancy.
> >
> > Also note that, according to Table 13-28 in RM (Rev 2 07/2022) for i.MX8M
> > Nano, VCO_out and Fout has a maximum of 1500MHz only. Although the note above
> > mentions a range of 1050-2100MHz...
>
> I looked up the limits in NXP's downstream kernel [1] , and I believe
> these values match the table in the reference manual instead of the
> conflicting sentence. I am guessing this is why the PMS values I get
> match those of the NXP downstream kernel.
>
> adam
>
> [1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/imx/sec_mipi_pll_1432x.h#L38
>
> >
> > Best regards,
> > Alexander
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.tq-group.com/
> >
> >

2023-04-20 21:55:48

by Adam Ford

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On Thu, Apr 20, 2023 at 8:43 AM Lucas Stach <[email protected]> wrote:
>
> Am Donnerstag, dem 20.04.2023 um 08:24 -0500 schrieb Adam Ford:
> > On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
> > >
> > > Hi Adam,
> > >
> > > Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> > > > On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> > > > >
> > > > > On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> > > > > >
> > > > > > Hi Adam,
> > > > > >
> > > > > > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > > > > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > > > > > bytes/pixel, then they are divided amongst the different lanes with some
> > > > > > > additional overhead. This is necessary to achieve higher resolutions while
> > > > > > > keeping the pixel clocks lower as the number of lanes increase.
> > > > > > >
> > > > > >
> > > > > > In the testing I did to come up with my patch "drm: bridge: samsung-
> > > > > > dsim: fix blanking packet size calculation" the number of lanes didn't
> > > > > > make any difference. My testing might be flawed, as I could only
> > > > > > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > > > > > interested to know what others did here. How did you validate the
> > > > > > blanking with your patch? Would you have a chance to test my patch and
> > > > > > see if it works or breaks in your setup?
> > > >
> > > > Lucas,
> > > >
> > > > I tried your patch instead of mine. Yours is dependent on the
> > > > hs_clock being always set to the burst clock which is configured by
> > > > the device tree. I unrolled a bit of my stuff and replaced it with
> > > > yours. It worked at 1080p, but when I tried a few other resolutions,
> > > > they did not work. I assume it's because the DSI clock is fixed and
> > > > not changing based on the pixel clock. In the version I did, I only
> > > > did that math when the lanes were > 1. In your patch, you divide by 8,
> > > > and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> > > > that just in case the bpp ever changes from 8. Overall, I think our
> > > > patches basically do the same thing.
> > >
> > > The calculations in your and my patch are quite different. I'm not
> > > taking into account the number of lanes or the MIPI format. I'm basing

I was taking the number of lanes into account in order to calculate
the clock rate, since 4-lanes can run slower.

> >
> > I was looking more at the division by 8 and the overhead correction of 6.
> > This number 6 also appears in the NXP downstream kernel [1]. I know
> > Marek V had some concerns about that.
> >
> Yea, I don't fully remember the details about the overhead. Need to
> page that back in. The division by 8 in my patch is just to get from
> the bit to a byte clock, nothing to do with the MIPI format bits per
> channel or something like that.
>
> > > the blanking size purely on the ratio between MIPI DSI byte clock and
> > > the DPI interface clock. It's quite counter-intuitive that the host
> > > would scale the blanking to the number of lanes automatically, but
> > > still require the MIPI packet offset removed, but that's what my
> > > measurements showed to produce the correct blanking after translation
> > > to DPI by the TC358767 bridge chip.
> >
> > How many lanes is your DSI interface using?
> >
> When I did the measurements to come up with the patch, I varied the
> number of lanes between 1 and 4. Different number of lanes didn't make
> a difference. In fact trying to compensate for the number of lanes when
> calculating the blanking size to program into the controller lead to
> wildly wrong blanking on the DPI side of the external bridge.
>
> > >
> > > If you dynamically scale the HS clock, then you would need to input the
> > > real used HS clock to the calculation in my patch, instead of the fixed
> > > burst mode rate.
> >
> > I think what you're saying makes sense.
> >
> > The code I originally modeled this from was from the NXP downstream
> > kernel where they define the calculation as being in words [2]. I am
> > not saying the NXP code is perfect, but the NXP code works. With this
> > series, my monitors are able to sync a bunch of different resolutions
> > from 1080p down to 640x480 and a bunch in between with various refresh
> > rates too. That was the goal of this series.
> >
> > Instead of just using your patch as-is, I will adapt yours to use the
> > scaled clock to see how it behaves and get back to you.
> >
>
> Thanks, that would be very much appreciated.

Lucas,

I took your patch and added a dsi state variable named "hs_clock" to
keep track of the output of samsung_dsim_set_pll which should be the
active high-speed clock.

I then replaced one line in your code to reference the hs_clock
instead of the burst clock:

@@ -960,7 +962,7 @@ static void samsung_dsim_set_display_mode(struct
samsung_dsim *dsi)
u32 reg;

if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
- int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
+ int byte_clk_khz = dsi->hs_clock / 1000 / 8;
int hfp = (m->hsync_start - m->hdisplay) *
byte_clk_khz / m->clock;

With that change, your patch works with the rest of my code, and I
think it's easier to read, and it doesn't involve recalculating the
clock speed each time since it's cached. If you're OK with that, I'll
incorporate your code into V2 of my series, and I'll apply my changes
as a subsequent patch. I hope to be able to send out V2 this weekend.

I would be curious to know frm Marek Szyprowski what the impact is on
the Samsung devices, if any.

adam

>
> I also don't assert that my calculation is perfect, as I also don't
> have any more information and needed to resort to observing the
> blanking after translation by the external bridge, so I hope we could
> get some better understanding of how things really work by checking
> what works for both our systems.
>
> > I have
> > finished reworking the dynamic DPHY settings, and I've fixed up making
> > the PLL device tree optional. If your patch works, I'll drop my
> > calculation and just build off what you have to use the scaled HS
> > clock when I submit the V2 and I'll make sure I CC you.
> >
> Thanks, I'm open to re-do my measurements with your new patches.
>
> Regards,
> Lucas
>
> > adam
> >
> > [1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L270
> > [2] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L914
> >
> > >
> > > Regards,
> > > Lucas
>

2023-04-21 08:43:19

by Lucas Stach

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

Am Donnerstag, dem 20.04.2023 um 16:51 -0500 schrieb Adam Ford:
> On Thu, Apr 20, 2023 at 8:43 AM Lucas Stach <[email protected]> wrote:
> >
> > Am Donnerstag, dem 20.04.2023 um 08:24 -0500 schrieb Adam Ford:
> > > On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
> > > >
> > > > Hi Adam,
> > > >
> > > > Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> > > > > On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> > > > > >
> > > > > > On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> > > > > > >
> > > > > > > Hi Adam,
> > > > > > >
> > > > > > > Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> > > > > > > > If there is more than one lane, the HFP, HBP, and HSA is calculated in
> > > > > > > > bytes/pixel, then they are divided amongst the different lanes with some
> > > > > > > > additional overhead. This is necessary to achieve higher resolutions while
> > > > > > > > keeping the pixel clocks lower as the number of lanes increase.
> > > > > > > >
> > > > > > >
> > > > > > > In the testing I did to come up with my patch "drm: bridge: samsung-
> > > > > > > dsim: fix blanking packet size calculation" the number of lanes didn't
> > > > > > > make any difference. My testing might be flawed, as I could only
> > > > > > > measure the blanking after translation from MIPI DSI to DPI, so I'm
> > > > > > > interested to know what others did here. How did you validate the
> > > > > > > blanking with your patch? Would you have a chance to test my patch and
> > > > > > > see if it works or breaks in your setup?
> > > > >
> > > > > Lucas,
> > > > >
> > > > > I tried your patch instead of mine. Yours is dependent on the
> > > > > hs_clock being always set to the burst clock which is configured by
> > > > > the device tree. I unrolled a bit of my stuff and replaced it with
> > > > > yours. It worked at 1080p, but when I tried a few other resolutions,
> > > > > they did not work. I assume it's because the DSI clock is fixed and
> > > > > not changing based on the pixel clock. In the version I did, I only
> > > > > did that math when the lanes were > 1. In your patch, you divide by 8,
> > > > > and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> > > > > that just in case the bpp ever changes from 8. Overall, I think our
> > > > > patches basically do the same thing.
> > > >
> > > > The calculations in your and my patch are quite different. I'm not
> > > > taking into account the number of lanes or the MIPI format. I'm basing
>
> I was taking the number of lanes into account in order to calculate
> the clock rate, since 4-lanes can run slower.
>
Ah that makes sense if you aren't running at full clock burst clock
rate.

> > >
> > > I was looking more at the division by 8 and the overhead correction of 6.
> > > This number 6 also appears in the NXP downstream kernel [1]. I know
> > > Marek V had some concerns about that.
> > >
> > Yea, I don't fully remember the details about the overhead. Need to
> > page that back in. The division by 8 in my patch is just to get from
> > the bit to a byte clock, nothing to do with the MIPI format bits per
> > channel or something like that.
> >
> > > > the blanking size purely on the ratio between MIPI DSI byte clock and
> > > > the DPI interface clock. It's quite counter-intuitive that the host
> > > > would scale the blanking to the number of lanes automatically, but
> > > > still require the MIPI packet offset removed, but that's what my
> > > > measurements showed to produce the correct blanking after translation
> > > > to DPI by the TC358767 bridge chip.
> > >
> > > How many lanes is your DSI interface using?
> > >
> > When I did the measurements to come up with the patch, I varied the
> > number of lanes between 1 and 4. Different number of lanes didn't make
> > a difference. In fact trying to compensate for the number of lanes when
> > calculating the blanking size to program into the controller lead to
> > wildly wrong blanking on the DPI side of the external bridge.
> >
> > > >
> > > > If you dynamically scale the HS clock, then you would need to input the
> > > > real used HS clock to the calculation in my patch, instead of the fixed
> > > > burst mode rate.
> > >
> > > I think what you're saying makes sense.
> > >
> > > The code I originally modeled this from was from the NXP downstream
> > > kernel where they define the calculation as being in words [2]. I am
> > > not saying the NXP code is perfect, but the NXP code works. With this
> > > series, my monitors are able to sync a bunch of different resolutions
> > > from 1080p down to 640x480 and a bunch in between with various refresh
> > > rates too. That was the goal of this series.
> > >
> > > Instead of just using your patch as-is, I will adapt yours to use the
> > > scaled clock to see how it behaves and get back to you.
> > >
> >
> > Thanks, that would be very much appreciated.
>
> Lucas,
>
> I took your patch and added a dsi state variable named "hs_clock" to
> keep track of the output of samsung_dsim_set_pll which should be the
> active high-speed clock.
>
> I then replaced one line in your code to reference the hs_clock
> instead of the burst clock:
>
> @@ -960,7 +962,7 @@ static void samsung_dsim_set_display_mode(struct
> samsung_dsim *dsi)
> u32 reg;
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
> + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
> int hfp = (m->hsync_start - m->hdisplay) *
> byte_clk_khz / m->clock;
>
> With that change, your patch works with the rest of my code, and I
> think it's easier to read, and it doesn't involve recalculating the
> clock speed each time since it's cached. If you're OK with that, I'll
> incorporate your code into V2 of my series, and I'll apply my changes
> as a subsequent patch. I hope to be able to send out V2 this weekend.
>
That's good to hear! Seems we are converging here. Feel free to pick up
the patch, that's also easier for me as I don't have to resend with CC
fixed.

> I would be curious to know frm Marek Szyprowski what the impact is on
> the Samsung devices, if any.
>
Since I messed up the list CC you also couldn't see his reply to my
patch:

| Tested-by: Marek Szyprowski <[email protected]>
|
| Works fine on the Exynos based boards I have in my test farm.

Regards,
Lucas

> adam
>
> >
> > I also don't assert that my calculation is perfect, as I also don't
> > have any more information and needed to resort to observing the
> > blanking after translation by the external bridge, so I hope we could
> > get some better understanding of how things really work by checking
> > what works for both our systems.
> >
> > > I have
> > > finished reworking the dynamic DPHY settings, and I've fixed up making
> > > the PLL device tree optional. If your patch works, I'll drop my
> > > calculation and just build off what you have to use the scaled HS
> > > clock when I submit the V2 and I'll make sure I CC you.
> > >
> > Thanks, I'm open to re-do my measurements with your new patches.
> >
> > Regards,
> > Lucas
> >
> > > adam
> > >
> > > [1] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L270
> > > [2] - https://github.com/nxp-imx/linux-imx/blob/lf-6.1.y/drivers/gpu/drm/bridge/sec-dsim.c#L914
> > >
> > > >
> > > > Regards,
> > > > Lucas
> >

2023-04-21 11:36:35

by Marek Szyprowski

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On 15.04.2023 12:41, Adam Ford wrote:
> Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> having an entry in the device tree for samsung,pll-clock-frequency.
>
> Signed-off-by: Adam Ford <[email protected]>

This one breaks DSI panel operation on my Exynos-based Trats, Trats2 and
TM2e boards. I've didn't check the details, but probably something is
missing in the dts to make it working properly. Surprisingly the display
is still working fine on Arndale board with DSI TC358764 bridge.

> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 9fec32b44e05..73f0c3fbbdf5 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> struct device_node *node = dev->of_node;
> int ret;
>
> - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> - &dsi->pll_clk_rate);
> - if (ret < 0)
> - return ret;
> -
> ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> &dsi->burst_clk_rate);
> if (ret < 0)
> @@ -1823,13 +1818,18 @@ int samsung_dsim_probe(struct platform_device *pdev)
> if (IS_ERR(dsi->clks[i])) {
> if (strcmp(clk_names[i], "sclk_mipi") == 0) {
> dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME);
> - if (!IS_ERR(dsi->clks[i]))
> + if (!IS_ERR(dsi->clks[i])) {
> + dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
> continue;
> + }
> }
>
> dev_info(dev, "failed to get the clock: %s\n", clk_names[i]);
> return PTR_ERR(dsi->clks[i]);
> }
> +
> + if (strcmp(clk_names[i], "sclk_mipi") == 0)
> + dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
> }
>
> dsi->reg_base = devm_platform_ioremap_resource(pdev, 0);

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2023-04-21 11:36:38

by Marek Szyprowski

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On 21.04.2023 10:40, Lucas Stach wrote:
> Am Donnerstag, dem 20.04.2023 um 16:51 -0500 schrieb Adam Ford:
>> On Thu, Apr 20, 2023 at 8:43 AM Lucas Stach <[email protected]> wrote:
>>> Am Donnerstag, dem 20.04.2023 um 08:24 -0500 schrieb Adam Ford:
>>>> On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
>>>>> Hi Adam,
>>>>>
>>>>> Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
>>>>>> On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
>>>>>>> On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
>>>>>>>> Hi Adam,
>>>>>>>>
>>>>>>>> Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
>>>>>>>>> If there is more than one lane, the HFP, HBP, and HSA is calculated in
>>>>>>>>> bytes/pixel, then they are divided amongst the different lanes with some
>>>>>>>>> additional overhead. This is necessary to achieve higher resolutions while
>>>>>>>>> keeping the pixel clocks lower as the number of lanes increase.
>>>>>>>>>
>>>>>>>> In the testing I did to come up with my patch "drm: bridge: samsung-
>>>>>>>> dsim: fix blanking packet size calculation" the number of lanes didn't
>>>>>>>> make any difference. My testing might be flawed, as I could only
>>>>>>>> measure the blanking after translation from MIPI DSI to DPI, so I'm
>>>>>>>> interested to know what others did here. How did you validate the
>>>>>>>> blanking with your patch? Would you have a chance to test my patch and
>>>>>>>> see if it works or breaks in your setup?
>>>>>> Lucas,
>>>>>>
>>>>>> I tried your patch instead of mine. Yours is dependent on the
>>>>>> hs_clock being always set to the burst clock which is configured by
>>>>>> the device tree. I unrolled a bit of my stuff and replaced it with
>>>>>> yours. It worked at 1080p, but when I tried a few other resolutions,
>>>>>> they did not work. I assume it's because the DSI clock is fixed and
>>>>>> not changing based on the pixel clock. In the version I did, I only
>>>>>> did that math when the lanes were > 1. In your patch, you divide by 8,
>>>>>> and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
>>>>>> that just in case the bpp ever changes from 8. Overall, I think our
>>>>>> patches basically do the same thing.
>>>>> The calculations in your and my patch are quite different. I'm not
>>>>> taking into account the number of lanes or the MIPI format. I'm basing
>> I was taking the number of lanes into account in order to calculate
>> the clock rate, since 4-lanes can run slower.
>>
> Ah that makes sense if you aren't running at full clock burst clock
> rate.
>
>>>> I was looking more at the division by 8 and the overhead correction of 6.
>>>> This number 6 also appears in the NXP downstream kernel [1]. I know
>>>> Marek V had some concerns about that.
>>>>
>>> Yea, I don't fully remember the details about the overhead. Need to
>>> page that back in. The division by 8 in my patch is just to get from
>>> the bit to a byte clock, nothing to do with the MIPI format bits per
>>> channel or something like that.
>>>
>>>>> the blanking size purely on the ratio between MIPI DSI byte clock and
>>>>> the DPI interface clock. It's quite counter-intuitive that the host
>>>>> would scale the blanking to the number of lanes automatically, but
>>>>> still require the MIPI packet offset removed, but that's what my
>>>>> measurements showed to produce the correct blanking after translation
>>>>> to DPI by the TC358767 bridge chip.
>>>> How many lanes is your DSI interface using?
>>>>
>>> When I did the measurements to come up with the patch, I varied the
>>> number of lanes between 1 and 4. Different number of lanes didn't make
>>> a difference. In fact trying to compensate for the number of lanes when
>>> calculating the blanking size to program into the controller lead to
>>> wildly wrong blanking on the DPI side of the external bridge.
>>>
>>>>> If you dynamically scale the HS clock, then you would need to input the
>>>>> real used HS clock to the calculation in my patch, instead of the fixed
>>>>> burst mode rate.
>>>> I think what you're saying makes sense.
>>>>
>>>> The code I originally modeled this from was from the NXP downstream
>>>> kernel where they define the calculation as being in words [2]. I am
>>>> not saying the NXP code is perfect, but the NXP code works. With this
>>>> series, my monitors are able to sync a bunch of different resolutions
>>>> from 1080p down to 640x480 and a bunch in between with various refresh
>>>> rates too. That was the goal of this series.
>>>>
>>>> Instead of just using your patch as-is, I will adapt yours to use the
>>>> scaled clock to see how it behaves and get back to you.
>>>>
>>> Thanks, that would be very much appreciated.
>> Lucas,
>>
>> I took your patch and added a dsi state variable named "hs_clock" to
>> keep track of the output of samsung_dsim_set_pll which should be the
>> active high-speed clock.
>>
>> I then replaced one line in your code to reference the hs_clock
>> instead of the burst clock:
>>
>> @@ -960,7 +962,7 @@ static void samsung_dsim_set_display_mode(struct
>> samsung_dsim *dsi)
>> u32 reg;
>>
>> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
>> - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
>> + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
>> int hfp = (m->hsync_start - m->hdisplay) *
>> byte_clk_khz / m->clock;
>>
>> With that change, your patch works with the rest of my code, and I
>> think it's easier to read, and it doesn't involve recalculating the
>> clock speed each time since it's cached. If you're OK with that, I'll
>> incorporate your code into V2 of my series, and I'll apply my changes
>> as a subsequent patch. I hope to be able to send out V2 this weekend.
>>
> That's good to hear! Seems we are converging here. Feel free to pick up
> the patch, that's also easier for me as I don't have to resend with CC
> fixed.
>
>> I would be curious to know frm Marek Szyprowski what the impact is on
>> the Samsung devices, if any.
>>
> Since I messed up the list CC you also couldn't see his reply to my
> patch:
>
> | Tested-by: Marek Szyprowski <[email protected]>
> |
> | Works fine on the Exynos based boards I have in my test farm.

I didn't follow this discussion, I'm a bit busy with other stuff. I've
just tested this series and patch #3 break Exynos based board. If you
want me to test anything else (might be a work-in-progress code), just
let me know by the separate mail.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2023-04-21 11:47:56

by Adam Ford

Subject: Re: [PATCH 1/6] drm: bridge: samsung-dsim: Support multi-lane calculations

On Fri, Apr 21, 2023 at 6:28 AM Marek Szyprowski
<[email protected]> wrote:
>
> On 21.04.2023 10:40, Lucas Stach wrote:
> > Am Donnerstag, dem 20.04.2023 um 16:51 -0500 schrieb Adam Ford:
> >> On Thu, Apr 20, 2023 at 8:43 AM Lucas Stach <[email protected]> wrote:
> >>> Am Donnerstag, dem 20.04.2023 um 08:24 -0500 schrieb Adam Ford:
> >>>> On Thu, Apr 20, 2023 at 8:06 AM Lucas Stach <[email protected]> wrote:
> >>>>> Hi Adam,
> >>>>>
> >>>>> Am Mittwoch, dem 19.04.2023 um 05:47 -0500 schrieb Adam Ford:
> >>>>>> On Mon, Apr 17, 2023 at 6:55 AM Adam Ford <[email protected]> wrote:
> >>>>>>> On Mon, Apr 17, 2023 at 3:43 AM Lucas Stach <[email protected]> wrote:
> >>>>>>>> Hi Adam,
> >>>>>>>>
> >>>>>>>> Am Samstag, dem 15.04.2023 um 05:40 -0500 schrieb Adam Ford:
> >>>>>>>>> If there is more than one lane, the HFP, HBP, and HSA is calculated in
> >>>>>>>>> bytes/pixel, then they are divided amongst the different lanes with some
> >>>>>>>>> additional overhead. This is necessary to achieve higher resolutions while
> >>>>>>>>> keeping the pixel clocks lower as the number of lanes increase.
> >>>>>>>>>
> >>>>>>>> In the testing I did to come up with my patch "drm: bridge: samsung-
> >>>>>>>> dsim: fix blanking packet size calculation" the number of lanes didn't
> >>>>>>>> make any difference. My testing might be flawed, as I could only
> >>>>>>>> measure the blanking after translation from MIPI DSI to DPI, so I'm
> >>>>>>>> interested to know what others did here. How did you validate the
> >>>>>>>> blanking with your patch? Would you have a chance to test my patch and
> >>>>>>>> see if it works or breaks in your setup?
> >>>>>> Lucas,
> >>>>>>
> >>>>>> I tried your patch instead of mine. Yours is dependent on the
> >>>>>> hs_clock being always set to the burst clock which is configured by
> >>>>>> the device tree. I unrolled a bit of my stuff and replaced it with
> >>>>>> yours. It worked at 1080p, but when I tried a few other resolutions,
> >>>>>> they did not work. I assume it's because the DSI clock is fixed and
> >>>>>> not changing based on the pixel clock. In the version I did, I only
> >>>>>> did that math when the lanes were > 1. In your patch, you divide by 8,
> >>>>>> and in mine, I fetch the bits-per-pixel (which is 8) and I divide by
> >>>>>> that just in case the bpp ever changes from 8. Overall, I think our
> >>>>>> patches basically do the same thing.
> >>>>> The calculations in your and my patch are quite different. I'm not
> >>>>> taking into account the number of lanes or the MIPI format. I'm basing
> >> I was taking the number of lanes into account in order to calculate
> >> the clock rate, since 4-lanes can run slower.
> >>
> > Ah that makes sense if you aren't running at full clock burst clock
> > rate.
> >
> >>>> I was looking more at the division by 8 and the overhead correction of 6.
> >>>> This number 6 also appears in the NXP downstream kernel [1]. I know
> >>>> Marek V had some concerns about that.
> >>>>
> >>> Yea, I don't fully remember the details about the overhead. Need to
> >>> page that back in. The division by 8 in my patch is just to get from
> >>> the bit to a byte clock, nothing to do with the MIPI format bits per
> >>> channel or something like that.
> >>>
> >>>>> the blanking size purely on the ratio between MIPI DSI byte clock and
> >>>>> the DPI interface clock. It's quite counter-intuitive that the host
> >>>>> would scale the blanking to the number of lanes automatically, but
> >>>>> still require the MIPI packet offset removed, but that's what my
> >>>>> measurements showed to produce the correct blanking after translation
> >>>>> to DPI by the TC358767 bridge chip.
> >>>> How many lanes is your DSI interface using?
> >>>>
> >>> When I did the measurements to come up with the patch, I varied the
> >>> number of lanes between 1 and 4. Different number of lanes didn't make
> >>> a difference. In fact trying to compensate for the number of lanes when
> >>> calculating the blanking size to program into the controller lead to
> >>> wildly wrong blanking on the DPI side of the external bridge.
> >>>
> >>>>> If you dynamically scale the HS clock, then you would need to input the
> >>>>> real used HS clock to the calculation in my patch, instead of the fixed
> >>>>> burst mode rate.
> >>>> I think what you're saying makes sense.
> >>>>
> >>>> The code I originally modeled this from was from the NXP downstream
> >>>> kernel where they define the calculation as being in words [2]. I am
> >>>> not saying the NXP code is perfect, but the NXP code works. With this
> >>>> series, my monitors are able to sync a bunch of different resolutions
> >>>> from 1080p down to 640x480 and a bunch in between with various refresh
> >>>> rates too. That was the goal of this series.
> >>>>
> >>>> Instead of just using your patch as-is, I will adapt yours to use the
> >>>> scaled clock to see how it behaves and get back to you.
> >>>>
> >>> Thanks, that would be very much appreciated.
> >> Lucas,
> >>
> >> I took your patch and added a dsi state variable named "hs_clock" to
> >> keep track of the output of samsung_dsim_set_pll which should be the
> >> active high-speed clock.
> >>
> >> I then replaced one line in your code to reference the hs_clock
> >> instead of the burst clock:
> >>
> >> @@ -960,7 +962,7 @@ static void samsung_dsim_set_display_mode(struct
> >> samsung_dsim *dsi)
> >> u32 reg;
> >>
> >> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> >> - int byte_clk_khz = dsi->burst_clk_rate / 1000 / 8;
> >> + int byte_clk_khz = dsi->hs_clock / 1000 / 8;
> >> int hfp = (m->hsync_start - m->hdisplay) *
> >> byte_clk_khz / m->clock;
> >>
> >> With that change, your patch works with the rest of my code, and I
> >> think it's easier to read, and it doesn't involve recalculating the
> >> clock speed each time since it's cached. If you're OK with that, I'll
> >> incorporate your code into V2 of my series, and I'll apply my changes
> >> as a subsequent patch. I hope to be able to send out V2 this weekend.
> >>
> > That's good to hear! Seems we are converging here. Feel free to pick up
> > the patch, that's also easier for me as I don't have to resend with CC
> > fixed.
> >
> >> I would be curious to know frm Marek Szyprowski what the impact is on
> >> the Samsung devices, if any.
> >>
> > Since I messed up the list CC you also couldn't see his reply to my
> > patch:
> >
> > | Tested-by: Marek Szyprowski <[email protected]>
> > |
> > | Works fine on the Exynos based boards I have in my test farm.
>
> I didn't follow this discussion, I'm a bit busy with other stuff. I've
> just tested this series and patch #3 break Exynos based board. If you
> want me to test anything else (might be a work-in-progress code), just
> let me know by the separate mail.

That's ok. I'm going to drop my patch in favor of Lucas' patch, since
you've already tested his, and it looks cleaner than mine. Thanks for
your willingness to test. That really helps us move forward without
breaking your stuff.

adam
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>

2023-04-21 11:49:30

by Adam Ford

Subject: Re: [PATCH 3/6] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically

On Fri, Apr 21, 2023 at 6:25 AM Marek Szyprowski
<[email protected]> wrote:
>
> On 15.04.2023 12:41, Adam Ford wrote:
> > Fetch the clock rate of "sclk_mipi" (or "pll_clk") instead of
> > having an entry in the device tree for samsung,pll-clock-frequency.
> >
> > Signed-off-by: Adam Ford <[email protected]>
>
> This one breaks DSI panel operation on my Exynos-based Trats, Trats2 and
> TM2e boards. I've didn't check the details, but probably something is
> missing in the dts to make it working properly. Surprisingly the display
> is still working fine on Arndale board with DSI TC358764 bridge.

Thanks for testing! I'm going to update this patch in V2 which will
use the device tree settings if they are present. If they are
missing, they will fetch the clock rate instead of failing. This way,
it should mitigate breaking your boards, but it will allow the imx8m
mini/nano/plus to eliminate hard-coding some device tree entries since
they can be fetched automatically.

adam
>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 9fec32b44e05..73f0c3fbbdf5 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -1744,11 +1744,6 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
> > struct device_node *node = dev->of_node;
> > int ret;
> >
> > - ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
> > - &dsi->pll_clk_rate);
> > - if (ret < 0)
> > - return ret;
> > -
> > ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
> > &dsi->burst_clk_rate);
> > if (ret < 0)
> > @@ -1823,13 +1818,18 @@ int samsung_dsim_probe(struct platform_device *pdev)
> > if (IS_ERR(dsi->clks[i])) {
> > if (strcmp(clk_names[i], "sclk_mipi") == 0) {
> > dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME);
> > - if (!IS_ERR(dsi->clks[i]))
> > + if (!IS_ERR(dsi->clks[i])) {
> > + dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
> > continue;
> > + }
> > }
> >
> > dev_info(dev, "failed to get the clock: %s\n", clk_names[i]);
> > return PTR_ERR(dsi->clks[i]);
> > }
> > +
> > + if (strcmp(clk_names[i], "sclk_mipi") == 0)
> > + dsi->pll_clk_rate = clk_get_rate(dsi->clks[i]);
> > }
> >
> > dsi->reg_base = devm_platform_ioremap_resource(pdev, 0);
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>