This series fixes the blanking pack size and the PMS calculation. It then
adds support to allows the DSIM to dynamically DPHY clocks, and support
non-burst mode while allowing the removal of the hard-coded clock values
for the PLL for imx8m mini/nano/plus, and it allows the removal of the
burst-clock device tree entry when burst-mode isn't supported by connected
devices like an HDMI brige. In that event, the HS clock is set to the
value requested by the bridge chip.
This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
Exynos boards.
Adam Ford (6):
drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
drm: bridge: samsung-dsim: Dynamically configure DPHY timing
drm: bridge: samsung-dsim: Support non-burst mode
dt-bindings: bridge: samsung-dsim: Make some flags optional
Lucas Stach (1):
drm: bridge: samsung-dsim: fix blanking packet size calculation
V8: Rebase. Add dt-bindings to series as Patch 7/7
V7: Move messages indicating the optional device tree items are going
to be automatically read elsewhere was move to dev_dbg instead of
dev_info. Cleaned up some of the comments to be a bit more clear.
Eliminated a double variable assignement accidentally introduced
in V6 when some of the items were moved from patch 6 to patch 5.
V6: Squash-in an additional error fix from Lucas Stach regarding the
DPHY calcuations. Remove the dynamic_dphy variable and let
everyone use the new calculations. Move the hs_clock caching
from patch 6 to patch 5 to go along with the DPHY calcuations
since they are now based on the recorded hs_clock rate.
V5: Update error message to dev_info and change them to indicate
what is happening without sounding like an error when optional
device tree entries are missing.
V4: Undo some accidental whitespace changes, rename PS_TO_CYCLE
variables to ps and hz from PS and MHz. Remove if check
before the samsung_dsim_set_phy_ctrl call since it's
unnecessary.
Added additional tested-by and reviewed-by comments.
Squash patches 6 and 7 together since the supporting
non-burst (patch 6) mode doesn't really work until
patch 7 was applied.
V3: When checking if the bust-clock is present, only check for it
in the device tree, and don't check the presence of the
MIPI_DSI_MODE_VIDEO_BURST flag as it breaks an existing Exynos
board.
Add a new patch to the series to select GENERIC_PHY_MIPI_DPHY in
Kconfig otherwise the build breaks on the 32-bit Exynos.
Change vco_min variable name to min_freq
Added tested-by from Chen-Yu Tsai
V2: Instead of using my packet blanking calculation, this integrates
on from Lucas Stach which gets modified later in the series to
cache the value of the HS-clock instead of having to do the
calucations again.
Instead of completely eliminating the PLL clock frequency from
the device tree, this makes it optional to avoid breaking some
Samsung devices. When the samsung,pll-clock-frequency is not
found, it reads the value of the clock named "sclk_mipi"
This also maintains backwards compatibility with older device
trees.
This also changes the DPHY calcuation from a Look-up table,
a reverse engineered algorithm which uses
phy_mipi_dphy_get_default_config to determine the standard
nominal values and calculates the cycles necessary to update
the DPHY timings accordingly.pu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/bridge/samsung-dsim.c | 141 +++++++++++++++---
include/drm/bridge/samsung-dsim.h | 4 +
4 files changed, 128 insertions(+), 27 deletions(-)
V8: Rebase onto the current master branch. Add dt-bindings to series.
V7: Move messages indicating the optional device tree items are going
to be automatically read elsewhere was move to dev_dbg instead of
dev_info. Cleaned up some of the comments to be a bit more clear.
Eliminated a double variable assignement accidentally introduced
in V6 when some of the items were moved from patch 6 to patch 5.
V6: Squash-in an additional error fix from Lucas Stach regarding the
DPHY calcuations. Remove the dynamic_dphy variable and let
everyone use the new calculations. Move the hs_clock caching
from patch 6 to patch 5 to go along with the DPHY calcuations
since they are now based on the recorded hs_clock rate.
V5: Update error message to dev_info and change them to indicate
what is happening without sounding like an error when optional
device tree entries are missing.
V4: Undo some accidental whitespace changes, rename PS_TO_CYCLE
variables to ps and hz from PS and MHz. Remove if check
before the samsung_dsim_set_phy_ctrl call since it's
unnecessary.
Added additional tested-by and reviewed-by comments.
Squash patches 6 and 7 together since the supporting
non-burst (patch 6) mode doesn't really work until
patch 7 was applied.
V3: When checking if the bust-clock is present, only check for it
in the device tree, and don't check the presence of the
MIPI_DSI_MODE_VIDEO_BURST flag as it breaks an existing Exynos
board.
Add a new patch to the series to select GENERIC_PHY_MIPI_DPHY in
Kconfig otherwise the build breaks on the 32-bit Exynos.
Change vco_min variable name to min_freq
Added tested-by from Chen-Yu Tsai
V2: Instead of using my packet blanking calculation, this integrates
on from Lucas Stach which gets modified later in the series to
cache the value of the HS-clock instead of having to do the
calucations again.
Instead of completely eliminating the PLL clock frequency from
the device tree, this makes it optional to avoid breaking some
Samsung devices. When the samsung,pll-clock-frequency is not
found, it reads the value of the clock named "sclk_mipi"
This also maintains backwards compatibility with older device
trees.
This also changes the DPHY calcuation from a Look-up table,
a reverse engineered algorithm which uses
phy_mipi_dphy_get_default_config to determine the standard
nominal values and calculates the cycles necessary to update
the DPHY timings accordingly.
--
2.39.2
From: Lucas Stach <[email protected]>
Scale the blanking packet sizes to match the ratio between HS clock
and DPI interface clock. The controller seems to do internal scaling
to the number of active lanes, so we don't take those into account.
Signed-off-by: Lucas Stach <[email protected]>
Signed-off-by: Adam Ford <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Reviewed-by: Jagan Teki <[email protected]>
Tested-by: Jagan Teki <[email protected]> # imx8mm-icore
---
drivers/gpu/drm/bridge/samsung-dsim.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 0f3f6846beea..a2d1eaf0ed1c 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -881,17 +881,29 @@ 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 hfp = (m->hsync_start - m->hdisplay) * byte_clk_khz / m->clock;
+ int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
+ int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
+
+ /* remove packet overhead when possible */
+ hfp = max(hfp - 6, 0);
+ hbp = max(hbp - 6, 0);
+ hsa = max(hsa - 6, 0);
+
+ dev_dbg(dsi->dev, "calculated hfp: %u, hbp: %u, hsa: %u",
+ hfp, hbp, hsa);
+
reg = DSIM_CMD_ALLOW(0xf)
| DSIM_STABLE_VFP(m->vsync_start - m->vdisplay)
| 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);
+ 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(m->hsync_end - m->hsync_start);
+ | DSIM_MAIN_HSA(hsa);
samsung_dsim_write(dsi, DSIM_MSYNC_REG, reg);
}
reg = DSIM_MAIN_HRESOL(m->hdisplay, num_bits_resol) |
--
2.39.2
In order to support variable DPHY timings, it's necessary
to enable GENERIC_PHY_MIPI_DPHY so phy_mipi_dphy_get_default_config
can be used to determine the nominal values for a given resolution
and refresh rate.
Signed-off-by: Adam Ford <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
---
drivers/gpu/drm/bridge/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index f076a09afac0..82c68b042444 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -227,6 +227,7 @@ config DRM_SAMSUNG_DSIM
select DRM_KMS_HELPER
select DRM_MIPI_DSI
select DRM_PANEL_BRIDGE
+ select GENERIC_PHY_MIPI_DPHY
help
The Samsung MIPI DSIM bridge controller driver.
This MIPI DSIM bridge can be found it on Exynos SoCs and
--
2.39.2
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 information was contradicted in other
parts of the mini, nano and plus manuals. After reaching out to my
NXP Rep, when confronting him about discrepencies in the Nano manual,
he responded with:
"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."
These updated values also match what is used in the NXP downstream
kernel.
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]>
Reviewed-by: Lucas Stach <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Reviewed-by: Jagan Teki <[email protected]>
Tested-by: Jagan Teki <[email protected]> # imx8mm-icore
---
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 a2d1eaf0ed1c..ead922c3ce9f 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -407,6 +407,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,
+ .min_freq = 500,
};
static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
@@ -420,6 +423,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,
+ .min_freq = 500,
};
static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
@@ -431,6 +437,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,
+ .min_freq = 500,
};
static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
@@ -443,6 +452,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,
+ .min_freq = 500,
};
static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
@@ -455,6 +467,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,
+ .min_freq = 500,
};
static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
@@ -471,6 +486,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,
+ .min_freq = 1050,
};
static const struct samsung_dsim_driver_data *
@@ -549,12 +567,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->min_freq * 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 6a37d1e079bf..2c20b9460c9a 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -54,11 +54,14 @@ struct samsung_dsim_driver_data {
unsigned int has_freqband:1;
unsigned int has_clklane_stop:1;
unsigned int num_clks;
+ unsigned int min_freq;
unsigned int max_freq;
unsigned int wait_for_reset;
unsigned int num_bits_resol;
unsigned int pll_p_offset;
const unsigned int *reg_values;
+ u16 m_min;
+ u16 m_max;
};
struct samsung_dsim_host_ops {
--
2.39.2
The DPHY timings are currently hard coded. Since the input
clock can be variable, the phy timings need to be variable
too. To facilitate this, we need to cache the hs_clock
based on what is generated from the PLL.
The phy_mipi_dphy_get_default_config_for_hsclk function
configures the DPHY timings in pico-seconds, and a small macro
converts those timings into clock cycles based on the hs_clk.
Signed-off-by: Adam Ford <[email protected]>
Signed-off-by: Lucas Stach <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
Tested-by: Michael Walle <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Reviewed-by: Jagan Teki <[email protected]>
Tested-by: Jagan Teki <[email protected]> # imx8mm-icore
---
drivers/gpu/drm/bridge/samsung-dsim.c | 58 +++++++++++++++++++++++----
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 307f1c20cfb9..41f557fee29a 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -220,6 +220,8 @@
#define OLD_SCLK_MIPI_CLK_NAME "pll_clk"
+#define PS_TO_CYCLE(ps, hz) DIV64_U64_ROUND_CLOSEST(((ps) * (hz)), 1000000000000ULL)
+
static const char *const clk_names[5] = {
"bus_clk",
"sclk_mipi",
@@ -658,6 +660,8 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
reg = samsung_dsim_read(dsi, DSIM_STATUS_REG);
} while ((reg & DSIM_PLL_STABLE) == 0);
+ dsi->hs_clock = fout;
+
return fout;
}
@@ -705,13 +709,47 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
const unsigned int *reg_values = driver_data->reg_values;
u32 reg;
+ struct phy_configure_opts_mipi_dphy cfg;
+ int clk_prepare, lpx, clk_zero, clk_post, clk_trail;
+ int hs_exit, hs_prepare, hs_zero, hs_trail;
+ unsigned long long byte_clock = dsi->hs_clock / 8;
if (driver_data->has_freqband)
return;
+ phy_mipi_dphy_get_default_config_for_hsclk(dsi->hs_clock,
+ dsi->lanes, &cfg);
+
+ /*
+ * TODO:
+ * The tech Applications Processor manuals for i.MX8M Mini, Nano,
+ * and Plus don't state what the definition of the PHYTIMING
+ * bits are beyond their address and bit position.
+ * After reviewing NXP's downstream code, it appears
+ * that the various PHYTIMING registers take the number
+ * of cycles and use various dividers on them. This
+ * calculation does not result in an exact match to the
+ * downstream code, but it is very close to the values
+ * generated by their lookup table, and it appears
+ * to sync at a variety of resolutions. If someone
+ * can get a more accurate mathematical equation needed
+ * for these registers, this should be updated.
+ */
+
+ lpx = PS_TO_CYCLE(cfg.lpx, byte_clock);
+ hs_exit = PS_TO_CYCLE(cfg.hs_exit, byte_clock);
+ clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, byte_clock);
+ clk_zero = PS_TO_CYCLE(cfg.clk_zero, byte_clock);
+ clk_post = PS_TO_CYCLE(cfg.clk_post, byte_clock);
+ clk_trail = PS_TO_CYCLE(cfg.clk_trail, byte_clock);
+ hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, byte_clock);
+ hs_zero = PS_TO_CYCLE(cfg.hs_zero, byte_clock);
+ hs_trail = PS_TO_CYCLE(cfg.hs_trail, byte_clock);
+
/* 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);
/*
@@ -719,7 +757,9 @@ 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];
+
+ reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit);
+
samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
/*
@@ -735,10 +775,11 @@ 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];
+
+ reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) |
+ DSIM_PHYTIMING1_CLK_ZERO(clk_zero) |
+ DSIM_PHYTIMING1_CLK_POST(clk_post) |
+ DSIM_PHYTIMING1_CLK_TRAIL(clk_trail);
samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg);
@@ -751,8 +792,11 @@ 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];
+
+ reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) |
+ DSIM_PHYTIMING2_HS_ZERO(hs_zero) |
+ DSIM_PHYTIMING2_HS_TRAIL(hs_trail);
+
samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg);
}
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index 2c20b9460c9a..05100e91ecb9 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -93,6 +93,7 @@ struct samsung_dsim {
u32 pll_clk_rate;
u32 burst_clk_rate;
+ u32 hs_clock;
u32 esc_clk_rate;
u32 lanes;
u32 mode_flags;
--
2.39.2
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.
This also removes the need to set a clock speed from
the device tree for non-burst mode operation, since the
pixel clock rate is the rate requested from the attached
device like a bridge chip. This should have no impact
for people using burst-mode and setting the burst clock
rate is still required for those users. If the burst
clock is not present, change the error message to
dev_info indicating the clock use the pixel clock.
Signed-off-by: Adam Ford <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Tested-by: Frieder Schrempf <[email protected]>
Reviewed-by: Frieder Schrempf <[email protected]>
Tested-by: Marek Szyprowski <[email protected]>
Reviewed-by: Jagan Teki <[email protected]>
Tested-by: Jagan Teki <[email protected]> # imx8mm-icore
---
drivers/gpu/drm/bridge/samsung-dsim.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 41f557fee29a..99ce2690582b 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -667,11 +667,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 if available, otherwise use the pix_clk */
+ if (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;
@@ -943,7 +953,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;
int hbp = (m->htotal - m->hsync_end) * byte_clk_khz / m->clock;
int hsa = (m->hsync_end - m->hsync_start) * byte_clk_khz / m->clock;
@@ -1794,10 +1804,13 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
return PTR_ERR(pll_clk);
}
+ /* If it doesn't exist, use pixel clock instead of failing */
ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
- &dsi->burst_clk_rate, 0);
- if (ret < 0)
- return ret;
+ &dsi->burst_clk_rate, 1);
+ if (ret < 0) {
+ dev_dbg(dev, "Using pixel clock for HS clock frequency\n");
+ dsi->burst_clk_rate = 0;
+ }
ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
&dsi->esc_clk_rate, 0);
--
2.39.2
In the event a device is connected to the samsung-dsim
controller that doesn't support the burst-clock, the
driver is able to get the requested pixel clock from the
attached device or bridge. In these instances, the
samsung,burst-clock-frequency isn't needed, so remove
it from the required list.
The pll-clock frequency can be set by the device tree entry
for samsung,pll-clock-frequency, but in some cases, the
pll-clock may have the same clock rate as sclk_mipi clock.
If they are equal, this flag is not needed since the driver
will use the sclk_mipi rate as a fallback.
Signed-off-by: Adam Ford <[email protected]>
---
.../bindings/display/bridge/samsung,mipi-dsim.yaml | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
index 9f61ebdfefa8..360fea81f4b6 100644
--- a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
@@ -70,7 +70,9 @@ properties:
samsung,burst-clock-frequency:
$ref: /schemas/types.yaml#/definitions/uint32
description:
- DSIM high speed burst mode frequency.
+ DSIM high speed burst mode frequency when connected to devices
+ that support burst mode. If absent, the driver will use the pixel
+ clock from the attached device or bridge.
samsung,esc-clock-frequency:
$ref: /schemas/types.yaml#/definitions/uint32
@@ -80,7 +82,8 @@ properties:
samsung,pll-clock-frequency:
$ref: /schemas/types.yaml#/definitions/uint32
description:
- DSIM oscillator clock frequency.
+ DSIM oscillator clock frequency. If absent, the driver will
+ use the clock frequency of sclk_mipi.
phys:
maxItems: 1
@@ -134,9 +137,7 @@ required:
- compatible
- interrupts
- reg
- - samsung,burst-clock-frequency
- samsung,esc-clock-frequency
- - samsung,pll-clock-frequency
allOf:
- $ref: ../dsi-controller.yaml#
--
2.39.2
Hi,
On Thu, 25 May 2023 22:05:52 -0500, Adam Ford wrote:
> This series fixes the blanking pack size and the PMS calculation. It then
> adds support to allows the DSIM to dynamically DPHY clocks, and support
> non-burst mode while allowing the removal of the hard-coded clock values
> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> burst-clock device tree entry when burst-mode isn't supported by connected
> devices like an HDMI brige. In that event, the HS clock is set to the
> value requested by the bridge chip.
>
> [...]
Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
[1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a617b33f7e513f25becf843bc97f8f1658c16337
[2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=54f1a83c72250b182fa7722b0c5f6eb5e769598d
[3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=33d8d14c83bf67aa0d262961a6fda9c40f3c1052
[4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=171b3b1e0f8b8c894f2388e1cf765a56f831ee5e
[5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89691775f5735fca9dc40e119edcbb52a25b9612
[6/7] drm: bridge: samsung-dsim: Support non-burst mode
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb0e13b9e223b218c9f242f8d340a332b4381042
[7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cfaf76d349837f695c8aa6d7077847fec4231fe5
--
Neil
On 26/05/2023 09:22, Neil Armstrong wrote:
> Hi,
>
> On Thu, 25 May 2023 22:05:52 -0500, Adam Ford wrote:
>> This series fixes the blanking pack size and the PMS calculation. It then
>> adds support to allows the DSIM to dynamically DPHY clocks, and support
>> non-burst mode while allowing the removal of the hard-coded clock values
>> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
>> burst-clock device tree entry when burst-mode isn't supported by connected
>> devices like an HDMI brige. In that event, the HS clock is set to the
>> value requested by the bridge chip.
>>
>> [...]
>
> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
>
> [1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a617b33f7e513f25becf843bc97f8f1658c16337
> [2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=54f1a83c72250b182fa7722b0c5f6eb5e769598d
> [3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=33d8d14c83bf67aa0d262961a6fda9c40f3c1052
> [4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=171b3b1e0f8b8c894f2388e1cf765a56f831ee5e
> [5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89691775f5735fca9dc40e119edcbb52a25b9612
> [6/7] drm: bridge: samsung-dsim: Support non-burst mode
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb0e13b9e223b218c9f242f8d340a332b4381042
> [7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cfaf76d349837f695c8aa6d7077847fec4231fe5
>
OK I made a bad manipulation, I applied patch 7 without review... I'll send a revert patch.
Neil
On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
<[email protected]> wrote:
>
> On 26/05/2023 09:22, Neil Armstrong wrote:
> > Hi,
> >
> > On Thu, 25 May 2023 22:05:52 -0500, Adam Ford wrote:
> >> This series fixes the blanking pack size and the PMS calculation. It then
> >> adds support to allows the DSIM to dynamically DPHY clocks, and support
> >> non-burst mode while allowing the removal of the hard-coded clock values
> >> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> >> burst-clock device tree entry when burst-mode isn't supported by connected
> >> devices like an HDMI brige. In that event, the HS clock is set to the
> >> value requested by the bridge chip.
> >>
> >> [...]
> >
> > Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
> >
> > [1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a617b33f7e513f25becf843bc97f8f1658c16337
> > [2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=54f1a83c72250b182fa7722b0c5f6eb5e769598d
> > [3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=33d8d14c83bf67aa0d262961a6fda9c40f3c1052
> > [4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=171b3b1e0f8b8c894f2388e1cf765a56f831ee5e
> > [5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89691775f5735fca9dc40e119edcbb52a25b9612
> > [6/7] drm: bridge: samsung-dsim: Support non-burst mode
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb0e13b9e223b218c9f242f8d340a332b4381042
> > [7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
> > https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cfaf76d349837f695c8aa6d7077847fec4231fe5
> >
>
> OK I made a bad manipulation, I applied patch 7 without review... I'll send a revert patch.
Sorry, I didn't mean to complicate things by adding the binding patch.
I added a note in the cover letter to indicate it, but I also
recognize that it contradicted my earlier email.
adam
>
> Neil
Adam, Neil,
I meant to get to this earlier today, but broken CI got in the way...
On Thu, May 25, 2023 at 10:05:59PM -0500, Adam Ford wrote:
> In the event a device is connected to the samsung-dsim
> controller that doesn't support the burst-clock, the
> driver is able to get the requested pixel clock from the
> attached device or bridge. In these instances, the
> samsung,burst-clock-frequency isn't needed, so remove
> it from the required list.
>
> The pll-clock frequency can be set by the device tree entry
> for samsung,pll-clock-frequency, but in some cases, the
> pll-clock may have the same clock rate as sclk_mipi clock.
> If they are equal, this flag is not needed since the driver
> will use the sclk_mipi rate as a fallback.
>
> Signed-off-by: Adam Ford <[email protected]>
> ---
> .../bindings/display/bridge/samsung,mipi-dsim.yaml | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> index 9f61ebdfefa8..360fea81f4b6 100644
> --- a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> @@ -70,7 +70,9 @@ properties:
> samsung,burst-clock-frequency:
> $ref: /schemas/types.yaml#/definitions/uint32
> description:
> - DSIM high speed burst mode frequency.
> + DSIM high speed burst mode frequency when connected to devices
> + that support burst mode. If absent, the driver will use the pixel
> + clock from the attached device or bridge.
I'd rather this description did not say anything about drivers.
How about:
If absent, the pixel clock from the attached device or bridge
will be used instead.
Or perhaps "must be used"? Ditto below.
Description aside, the removal seems to be backwards compatible - but
can every device that this binding supports work using an "attached
device or bridge", or are these properties going to be required for
certain compatibles?
Thanks,
Conor.
>
> samsung,esc-clock-frequency:
> $ref: /schemas/types.yaml#/definitions/uint32
> @@ -80,7 +82,8 @@ properties:
> samsung,pll-clock-frequency:
> $ref: /schemas/types.yaml#/definitions/uint32
> description:
> - DSIM oscillator clock frequency.
> + DSIM oscillator clock frequency. If absent, the driver will
> + use the clock frequency of sclk_mipi.
>
> phys:
> maxItems: 1
> @@ -134,9 +137,7 @@ required:
> - compatible
> - interrupts
> - reg
> - - samsung,burst-clock-frequency
> - samsung,esc-clock-frequency
> - - samsung,pll-clock-frequency
>
> allOf:
> - $ref: ../dsi-controller.yaml#
> --
> 2.39.2
>
On Fri, May 26, 2023 at 1:19 PM Conor Dooley <[email protected]> wrote:
>
> Adam, Neil,
>
> I meant to get to this earlier today, but broken CI got in the way...
>
> On Thu, May 25, 2023 at 10:05:59PM -0500, Adam Ford wrote:
> > In the event a device is connected to the samsung-dsim
> > controller that doesn't support the burst-clock, the
> > driver is able to get the requested pixel clock from the
> > attached device or bridge. In these instances, the
> > samsung,burst-clock-frequency isn't needed, so remove
> > it from the required list.
> >
> > The pll-clock frequency can be set by the device tree entry
> > for samsung,pll-clock-frequency, but in some cases, the
> > pll-clock may have the same clock rate as sclk_mipi clock.
> > If they are equal, this flag is not needed since the driver
> > will use the sclk_mipi rate as a fallback.
> >
> > Signed-off-by: Adam Ford <[email protected]>
> > ---
> > .../bindings/display/bridge/samsung,mipi-dsim.yaml | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> > index 9f61ebdfefa8..360fea81f4b6 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/samsung,mipi-dsim.yaml
> > @@ -70,7 +70,9 @@ properties:
> > samsung,burst-clock-frequency:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > description:
> > - DSIM high speed burst mode frequency.
> > + DSIM high speed burst mode frequency when connected to devices
> > + that support burst mode. If absent, the driver will use the pixel
> > + clock from the attached device or bridge.
>
> I'd rather this description did not say anything about drivers.
> How about:
> If absent, the pixel clock from the attached device or bridge
> will be used instead.
That makes sense. I can do that.
"DSIM high speed burst mode frequency (optional). If absent, the pixel
clock from the attached device or bridge will be used instead."
> Or perhaps "must be used"? Ditto below.
"Must be" implies to me that the user needs to set something. Are you
ok with the proposed suggestion above?
>
> Description aside, the removal seems to be backwards compatible - but
> can every device that this binding supports work using an "attached
> device or bridge", or are these properties going to be required for
> certain compatibles?
From what I can tell, the assumption is that the DSIM driver was
expecting it to attach to panels in the past. With the additional
patch series, the DSIM can attach to bridge parts without a hard-coded
set of clocks. I don't expect the existing Exynos devices to change,
but I also don't know what would preclude those SoC's from attaching
to a bridge should someone want to design a new product around them.
I'll wait a couple days for more feedback and send patch V2 with just
this patch since the rest of the series has been applied to the drm
branch.
adam
>
> Thanks,
> Conor.
>
> >
> > samsung,esc-clock-frequency:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > @@ -80,7 +82,8 @@ properties:
> > samsung,pll-clock-frequency:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > description:
> > - DSIM oscillator clock frequency.
> > + DSIM oscillator clock frequency. If absent, the driver will
> > + use the clock frequency of sclk_mipi.
> >
> > phys:
> > maxItems: 1
> > @@ -134,9 +137,7 @@ required:
> > - compatible
> > - interrupts
> > - reg
> > - - samsung,burst-clock-frequency
> > - samsung,esc-clock-frequency
> > - - samsung,pll-clock-frequency
> >
> > allOf:
> > - $ref: ../dsi-controller.yaml#
> > --
> > 2.39.2
> >
On Fri, May 26, 2023 at 02:24:21PM -0500, Adam Ford wrote:
> On Fri, May 26, 2023 at 1:19 PM Conor Dooley <[email protected]> wrote:
> > On Thu, May 25, 2023 at 10:05:59PM -0500, Adam Ford wrote:
> > > description:
> > > - DSIM high speed burst mode frequency.
> > > + DSIM high speed burst mode frequency when connected to devices
> > > + that support burst mode. If absent, the driver will use the pixel
> > > + clock from the attached device or bridge.
> >
> > I'd rather this description did not say anything about drivers.
> > How about:
> > If absent, the pixel clock from the attached device or bridge
> > will be used instead.
>
> That makes sense. I can do that.
>
> "DSIM high speed burst mode frequency (optional). If absent, the pixel
> clock from the attached device or bridge will be used instead."
>
> > Or perhaps "must be used"? Ditto below.
>
> "Must be" implies to me that the user needs to set something. Are you
> ok with the proposed suggestion above?
> >
> > Description aside, the removal seems to be backwards compatible - but
> > can every device that this binding supports work using an "attached
> > device or bridge", or are these properties going to be required for
> > certain compatibles?
>
> From what I can tell, the assumption is that the DSIM driver was
> expecting it to attach to panels in the past. With the additional
> patch series, the DSIM can attach to bridge parts without a hard-coded
> set of clocks. I don't expect the existing Exynos devices to change,
> but I also don't know what would preclude those SoC's from attaching
> to a bridge should someone want to design a new product around them.
Okay, that seems fair. With your revised wording,
Reviewed-by: Conor Dooley <[email protected]>
>
> I'll wait a couple days for more feedback and send patch V2 with just
> this patch since the rest of the series has been applied to the drm
> branch.
Sounds good. Krzysztof will hopefully be able to take a look then too to
make sure I am not making a hames of things.
Thanks,
Conor.
On 26/05/2023 16:04, Adam Ford wrote:
> On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
> <[email protected]> wrote:
>>
>> On 26/05/2023 09:22, Neil Armstrong wrote:
>>> Hi,
>>>
>>> On Thu, 25 May 2023 22:05:52 -0500, Adam Ford wrote:
>>>> This series fixes the blanking pack size and the PMS calculation. It then
>>>> adds support to allows the DSIM to dynamically DPHY clocks, and support
>>>> non-burst mode while allowing the removal of the hard-coded clock values
>>>> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
>>>> burst-clock device tree entry when burst-mode isn't supported by connected
>>>> devices like an HDMI brige. In that event, the HS clock is set to the
>>>> value requested by the bridge chip.
>>>>
>>>> [...]
>>>
>>> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
>>>
>>> [1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a617b33f7e513f25becf843bc97f8f1658c16337
>>> [2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=54f1a83c72250b182fa7722b0c5f6eb5e769598d
>>> [3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=33d8d14c83bf67aa0d262961a6fda9c40f3c1052
>>> [4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=171b3b1e0f8b8c894f2388e1cf765a56f831ee5e
>>> [5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89691775f5735fca9dc40e119edcbb52a25b9612
>>> [6/7] drm: bridge: samsung-dsim: Support non-burst mode
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb0e13b9e223b218c9f242f8d340a332b4381042
>>> [7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cfaf76d349837f695c8aa6d7077847fec4231fe5
>>>
>>
>> OK I made a bad manipulation, I applied patch 7 without review... I'll send a revert patch.
>
> Sorry, I didn't mean to complicate things by adding the binding patch.
> I added a note in the cover letter to indicate it, but I also
> recognize that it contradicted my earlier email.
No problem :-)
Neil
>
> adam
>>
>> Neil
On 26/05/2023 21:30, Conor Dooley wrote:
> On Fri, May 26, 2023 at 02:24:21PM -0500, Adam Ford wrote:
>> On Fri, May 26, 2023 at 1:19 PM Conor Dooley <[email protected]> wrote:
>>> On Thu, May 25, 2023 at 10:05:59PM -0500, Adam Ford wrote:
>
>>>> description:
>>>> - DSIM high speed burst mode frequency.
>>>> + DSIM high speed burst mode frequency when connected to devices
>>>> + that support burst mode. If absent, the driver will use the pixel
>>>> + clock from the attached device or bridge.
>>>
>>> I'd rather this description did not say anything about drivers.
>>> How about:
>>> If absent, the pixel clock from the attached device or bridge
>>> will be used instead.
>>
>> That makes sense. I can do that.
>>
>> "DSIM high speed burst mode frequency (optional). If absent, the pixel
>> clock from the attached device or bridge will be used instead."
>>
>>> Or perhaps "must be used"? Ditto below.
>>
>> "Must be" implies to me that the user needs to set something. Are you
>> ok with the proposed suggestion above?
>>>
>>> Description aside, the removal seems to be backwards compatible - but
>>> can every device that this binding supports work using an "attached
>>> device or bridge", or are these properties going to be required for
>>> certain compatibles?
>>
>> From what I can tell, the assumption is that the DSIM driver was
>> expecting it to attach to panels in the past. With the additional
>> patch series, the DSIM can attach to bridge parts without a hard-coded
>> set of clocks. I don't expect the existing Exynos devices to change,
>> but I also don't know what would preclude those SoC's from attaching
>> to a bridge should someone want to design a new product around them.
>
> Okay, that seems fair. With your revised wording,
> Reviewed-by: Conor Dooley <[email protected]>
>
>>
>> I'll wait a couple days for more feedback and send patch V2 with just
>> this patch since the rest of the series has been applied to the drm
>> branch.
>
> Sounds good. Krzysztof will hopefully be able to take a look then too to
> make sure I am not making a hames of things.
We should avoid references to driver, because bindings are used also in
other projects where driver can behave differently. Also "driver" is
then ambiguous - which driver do you mean? Please re-phrase.
Best regards,
Krzysztof
On 26/05/2023 05.05, Adam Ford wrote:
> This series fixes the blanking pack size and the PMS calculation. It then
> adds support to allows the DSIM to dynamically DPHY clocks, and support
> non-burst mode while allowing the removal of the hard-coded clock values
> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> burst-clock device tree entry when burst-mode isn't supported by connected
> devices like an HDMI brige. In that event, the HS clock is set to the
> value requested by the bridge chip.
>
> This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
> work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
> Exynos boards.
Hi all
We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
1920x1200, but the monitor says it's only at 58Hz, and measuring on the
DSI signals does seem to confirm that the update frequency is about 57.7
or 57.8Hz (it's pretty hard to get a good measurement). It looks like
it's the lines that are too long, by a time that corresponds to about 80
pixels. But all the frontporch/backporch/hsync values look sane and
completely standard for that resolution.
Setting samsung,burst-clock-frequency explicitly to something large
enough or letting it be derived from the 154MHz pixel clock makes no
difference.
Any ideas?
Thanks,
Rasmus
On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 26/05/2023 05.05, Adam Ford wrote:
> > This series fixes the blanking pack size and the PMS calculation. It then
> > adds support to allows the DSIM to dynamically DPHY clocks, and support
> > non-burst mode while allowing the removal of the hard-coded clock values
> > for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> > burst-clock device tree entry when burst-mode isn't supported by connected
> > devices like an HDMI brige. In that event, the HS clock is set to the
> > value requested by the bridge chip.
> >
> > This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
> > work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
> > Exynos boards.
>
> Hi all
>
> We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
> ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
> 1920x1200, but the monitor says it's only at 58Hz, and measuring on the
> DSI signals does seem to confirm that the update frequency is about 57.7
> or 57.8Hz (it's pretty hard to get a good measurement). It looks like
> it's the lines that are too long, by a time that corresponds to about 80
> pixels. But all the frontporch/backporch/hsync values look sane and
> completely standard for that resolution.
>
> Setting samsung,burst-clock-frequency explicitly to something large
> enough or letting it be derived from the 154MHz pixel clock makes no
> difference.
>
> Any ideas?
What refresh rate are you trying to achieve? It seems like 57.7 or
57.8 is really close to the 58 the Monitor states. I would expect the
refresh to be driven by whatever the monitor states it can handle.
Have you tried using modetest to see what refresh rates are available?
When I was doing this driver work, I would use modetest to determine
the connector ID, then use modetest -s
<connector-id>:<resolution>-<refresh> to display various resolutions
and refresh rates.
The 8MP shares the video-pll clock with both disp1 and disp2 clocks,
and the imx-lcdif driver, which sends the display signals to the DSI,
uses the disp clock, so the video-pll needs to be an exact multiple of
the pixel clock or the output won't sink. Modetest should also show
you the desired pixel clock for a given resolution and refresh.
My displays didn't show 19200x1200 as an option, so I wasn't able to
test that configuration.
adam
>
> Thanks,
> Rasmus
>
On Wed, Jun 7, 2023 at 8:27 AM Adam Ford <[email protected]> wrote:
>
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <[email protected]> wrote:
> >
> > On 26/05/2023 05.05, Adam Ford wrote:
> > > This series fixes the blanking pack size and the PMS calculation. It then
> > > adds support to allows the DSIM to dynamically DPHY clocks, and support
> > > non-burst mode while allowing the removal of the hard-coded clock values
> > > for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> > > burst-clock device tree entry when burst-mode isn't supported by connected
> > > devices like an HDMI brige. In that event, the HS clock is set to the
> > > value requested by the bridge chip.
> > >
> > > This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
> > > work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
> > > Exynos boards.
> >
> > Hi all
> >
> > We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
> > ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
> > 1920x1200, but the monitor says it's only at 58Hz, and measuring on the
> > DSI signals does seem to confirm that the update frequency is about 57.7
> > or 57.8Hz (it's pretty hard to get a good measurement). It looks like
> > it's the lines that are too long, by a time that corresponds to about 80
> > pixels. But all the frontporch/backporch/hsync values look sane and
> > completely standard for that resolution.
> >
> > Setting samsung,burst-clock-frequency explicitly to something large
> > enough or letting it be derived from the 154MHz pixel clock makes no
> > difference.
> >
> > Any ideas?
>
> What refresh rate are you trying to achieve? It seems like 57.7 or
> 57.8 is really close to the 58 the Monitor states. I would expect the
> refresh to be driven by whatever the monitor states it can handle.
>
> Have you tried using modetest to see what refresh rates are available?
> When I was doing this driver work, I would use modetest to determine
> the connector ID, then use modetest -s
> <connector-id>:<resolution>-<refresh> to display various resolutions
> and refresh rates.
>
> The 8MP shares the video-pll clock with both disp1 and disp2 clocks,
> and the imx-lcdif driver, which sends the display signals to the DSI,
> uses the disp clock, so the video-pll needs to be an exact multiple of
> the pixel clock or the output won't sink. Modetest should also show
> you the desired pixel clock for a given resolution and refresh.
> My displays didn't show 19200x1200 as an option, so I wasn't able to
> test that configuration.
Another thing you could try would be this rounding patch that I'm
experimenting with [1].
From what I can see, some resolutions end up with math that end up
rounding down, and this patch corrects the timings a bit to attempt to
compensate. I haven't tested this extensively yet, but you can try it
to see if it helps.
adam
[1] - https://github.com/aford173/linux/commit/183cf6d154afeb9b0300500b09d7b8ec53047a12
>
> adam
> >
> > Thanks,
> > Rasmus
> >
On 07/06/2023 15.27, Adam Ford wrote:
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <[email protected]> wrote:
>>
>> On 26/05/2023 05.05, Adam Ford wrote:
>>> This series fixes the blanking pack size and the PMS calculation. It then
>>> adds support to allows the DSIM to dynamically DPHY clocks, and support
>>> non-burst mode while allowing the removal of the hard-coded clock values
>>> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
>>> burst-clock device tree entry when burst-mode isn't supported by connected
>>> devices like an HDMI brige. In that event, the HS clock is set to the
>>> value requested by the bridge chip.
>>>
>>> This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
>>> work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
>>> Exynos boards.
>>
>> Hi all
>>
>> We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
>> ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
>> 1920x1200, but the monitor says it's only at 58Hz, and measuring on the
>> DSI signals does seem to confirm that the update frequency is about 57.7
>> or 57.8Hz (it's pretty hard to get a good measurement). It looks like
>> it's the lines that are too long, by a time that corresponds to about 80
>> pixels. But all the frontporch/backporch/hsync values look sane and
>> completely standard for that resolution.
>>
>> Setting samsung,burst-clock-frequency explicitly to something large
>> enough or letting it be derived from the 154MHz pixel clock makes no
>> difference.
>>
>> Any ideas?
>
> What refresh rate are you trying to achieve? It seems like 57.7 or
> 57.8 is really close to the 58 the Monitor states.
Oh, sorry, I thought that was clear, but it should be/we're aiming
for/expecting 60Hz, or (154MHz / (2080 * 1235)) which is about 59.95Hz.
We've tried with a variety of monitors that all have 1920x1200@60Hz as
max resolution, and parse-edid always gives the same hfp/hbp/...
numbers, namely
Modeline "Mode 0" 154.00 1920 1968 2000 2080 1200 1203
1209 1235 +hsync -vsync
> I would expect the
> refresh to be driven by whatever the monitor states it can handle.
Well, it states that it can handle 60Hz, and the pixel clock is also
computed to be the 154MHz, but still, the actual signals on the wire,
and hence also what the monitor ends up reporting, do not end up with 60
full frames per second.
> Have you tried using modetest to see what refresh rates are available?
Hm. My userspace may be a little weird. When I run modetest I just get
trying to open device 'i915'...failed
trying to open device 'amdgpu'...failed
...
trying to open device 'imx-dcss'...failed
trying to open device 'mxsfb-drm'...failed
no device found
> The 8MP shares the video-pll clock with both disp1 and disp2 clocks,
> and the imx-lcdif driver, which sends the display signals to the DSI,
> uses the disp clock, so the video-pll needs to be an exact multiple of
> the pixel clock or the output won't sink.
Bingo! I enabled the
DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
in drivers/gpu/drm/mxsfb/lcdif_kms.c, and indeed it got me
Pixel clock: 154000kHz (actual: 148500kHz)
Modifying the 1039500000 in imx8mp.dtsi to 1078000000 (i.e. 7 times the
desired pixel clock) gave me "actual" matching the desired pixel clock,
and the monitor now reports 60Hz.
This product also has an LVDS display on lcdif2, so I'll have to
investigate how changing the video_pll1 rate affects that. And also what
to do about the case where somebody plugs in, say, a 1080p monitor that
would indeed require 148.5MHz pixel clock.
Thanks,
Rasmus
On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 07/06/2023 15.27, Adam Ford wrote:
> > On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> > <[email protected]> wrote:
> >>
> >> On 26/05/2023 05.05, Adam Ford wrote:
> >>> This series fixes the blanking pack size and the PMS calculation. It then
> >>> adds support to allows the DSIM to dynamically DPHY clocks, and support
> >>> non-burst mode while allowing the removal of the hard-coded clock values
> >>> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> >>> burst-clock device tree entry when burst-mode isn't supported by connected
> >>> devices like an HDMI brige. In that event, the HS clock is set to the
> >>> value requested by the bridge chip.
> >>>
> >>> This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
> >>> work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
> >>> Exynos boards.
> >>
> >> Hi all
> >>
> >> We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
> >> ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
> >> 1920x1200, but the monitor says it's only at 58Hz, and measuring on the
> >> DSI signals does seem to confirm that the update frequency is about 57.7
> >> or 57.8Hz (it's pretty hard to get a good measurement). It looks like
> >> it's the lines that are too long, by a time that corresponds to about 80
> >> pixels. But all the frontporch/backporch/hsync values look sane and
> >> completely standard for that resolution.
> >>
> >> Setting samsung,burst-clock-frequency explicitly to something large
> >> enough or letting it be derived from the 154MHz pixel clock makes no
> >> difference.
> >>
> >> Any ideas?
> >
> > What refresh rate are you trying to achieve? It seems like 57.7 or
> > 57.8 is really close to the 58 the Monitor states.
>
> Oh, sorry, I thought that was clear, but it should be/we're aiming
> for/expecting 60Hz, or (154MHz / (2080 * 1235)) which is about 59.95Hz.
> We've tried with a variety of monitors that all have 1920x1200@60Hz as
> max resolution, and parse-edid always gives the same hfp/hbp/...
> numbers, namely
>
> Modeline "Mode 0" 154.00 1920 1968 2000 2080 1200 1203
> 1209 1235 +hsync -vsync
>
> > I would expect the
> > refresh to be driven by whatever the monitor states it can handle.
>
> Well, it states that it can handle 60Hz, and the pixel clock is also
> computed to be the 154MHz, but still, the actual signals on the wire,
> and hence also what the monitor ends up reporting, do not end up with 60
> full frames per second.
>
> > Have you tried using modetest to see what refresh rates are available?
>
> Hm. My userspace may be a little weird. When I run modetest I just get
>
> trying to open device 'i915'...failed
> trying to open device 'amdgpu'...failed
> ...
> trying to open device 'imx-dcss'...failed
> trying to open device 'mxsfb-drm'...failed
> no device found
>
One the 8MP, I think you need to append "-M imx-lcdif" to the modetest
command to specify the driver being used.
I don't have my 8MP with me right now, but I think that's the right name.
> > The 8MP shares the video-pll clock with both disp1 and disp2 clocks,
> > and the imx-lcdif driver, which sends the display signals to the DSI,
> > uses the disp clock, so the video-pll needs to be an exact multiple of
> > the pixel clock or the output won't sink.
>
> Bingo! I enabled the
>
> DRM_DEV_DEBUG_DRIVER(drm->dev, "Pixel clock: %dkHz (actual: %dkHz)\n",
>
> in drivers/gpu/drm/mxsfb/lcdif_kms.c, and indeed it got me
>
> Pixel clock: 154000kHz (actual: 148500kHz)
>
> Modifying the 1039500000 in imx8mp.dtsi to 1078000000 (i.e. 7 times the
> desired pixel clock) gave me "actual" matching the desired pixel clock,
> and the monitor now reports 60Hz.
I am glad that worked!
>
> This product also has an LVDS display on lcdif2, so I'll have to
> investigate how changing the video_pll1 rate affects that. And also what
> to do about the case where somebody plugs in, say, a 1080p monitor that
> would indeed require 148.5MHz pixel clock.
That's the down-side to the 8MP with the shared clock. According to
the processor reference manual, It looks like the MEDIA_LDB_CLK can be
a child of Audio_PLL2. i don't know if you need both AUDIO_PLL1 and
Audio_PLL2, but the Audio_PLL2 clock is fairly flexible, so if you can
use Audio_pll1 for all your audio needs, and configure the audio_pll2
for your LVDS, you might be able to get both LDB and DSI to sync at
the nominal values.
adam
>
> Thanks,
> Rasmus
>
Am Donnerstag, dem 08.06.2023 um 07:30 -0500 schrieb Adam Ford:
> On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
> <[email protected]> wrote:
> >
> >
[...]
> > > Have you tried using modetest to see what refresh rates are available?
> >
> > Hm. My userspace may be a little weird. When I run modetest I just get
> >
> > trying to open device 'i915'...failed
> > trying to open device 'amdgpu'...failed
> > ...
> > trying to open device 'imx-dcss'...failed
> > trying to open device 'mxsfb-drm'...failed
> > no device found
> >
>
> One the 8MP, I think you need to append "-M imx-lcdif" to the modetest
> command to specify the driver being used.
> I don't have my 8MP with me right now, but I think that's the right name.
That's correct. Alternatively update libdrm to >= 2.4.114, which knows
to look for this driver in the tests.
Regards,
Lucas