This series fixes the blanking packet size and the PMS calculation.
According to Lucas, "The blanking packets are MIPI long packets,
so 4 byte header, payload, 2 bytes footer."
It also dds 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 a 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 based on feedback from other
users.
Adam Ford (5):
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
Lucas Stach (1):
drm: bridge: samsung-dsim: fix blanking packet size calculation
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/bridge/samsung-dsim.c | 160 ++++++++++++++++++++++----
include/drm/bridge/samsung-dsim.h | 5 +
3 files changed, 143 insertions(+), 23 deletions(-)
---
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 backwords 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
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.
Lastly, cache the clock rate configured from
samsung_dsim_set_pll in order to properly calculate
the blanking regardless of whether or not the burst
clock is set.
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]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 27 +++++++++++++++++++++------
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index d19a5c87b749..97872ffb903d 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -654,16 +654,28 @@ 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;
}
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;
@@ -952,7 +964,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;
@@ -1802,10 +1814,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_info(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);
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index 76ea8a1720cc..14176e6e9040 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -94,6 +94,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
Make the pll-clock-frequency optional. If it's present, use it
to maintain backwards compatibility with existing hardware. If it
is absent, read clock rate of "sclk_mipi" to determine the rate.
Since it can be optional, change the message from an error to
dev_info.
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]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index bf4b33d2de76..08266303c261 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1712,11 +1712,11 @@ static const struct mipi_dsi_host_ops samsung_dsim_ops = {
};
static int samsung_dsim_of_read_u32(const struct device_node *np,
- const char *propname, u32 *out_value)
+ const char *propname, u32 *out_value, bool optional)
{
int ret = of_property_read_u32(np, propname, out_value);
- if (ret < 0)
+ if (ret < 0 && !optional)
pr_err("%pOF: failed to get '%s' property\n", np, propname);
return ret;
@@ -1726,20 +1726,29 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
{
struct device *dev = dsi->dev;
struct device_node *node = dev->of_node;
+ struct clk *pll_clk;
int ret;
ret = samsung_dsim_of_read_u32(node, "samsung,pll-clock-frequency",
- &dsi->pll_clk_rate);
- if (ret < 0)
- return ret;
+ &dsi->pll_clk_rate, 1);
+
+ /* If it doesn't exist, read it from the clock instead of failing */
+ if (ret < 0) {
+ dev_info(dev, "Using sclk_mipi for pll clock frequency\n");
+ pll_clk = devm_clk_get(dev, "sclk_mipi");
+ if (!IS_ERR(pll_clk))
+ dsi->pll_clk_rate = clk_get_rate(pll_clk);
+ else
+ return PTR_ERR(pll_clk);
+ }
ret = samsung_dsim_of_read_u32(node, "samsung,burst-clock-frequency",
- &dsi->burst_clk_rate);
+ &dsi->burst_clk_rate, 0);
if (ret < 0)
return ret;
ret = samsung_dsim_of_read_u32(node, "samsung,esc-clock-frequency",
- &dsi->esc_clk_rate);
+ &dsi->esc_clk_rate, 0);
if (ret < 0)
return ret;
--
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. Add an additional variable to the driver data to enable
this feature to prevent breaking boards that don't support it.
The phy_mipi_dphy_get_default_config function configures the
DPHY timings in pico-seconds, and a small macro converts those
timings into clock cycles based on the pixel clock rate.
Signed-off-by: Adam Ford <[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]>
---
drivers/gpu/drm/bridge/samsung-dsim.c | 74 ++++++++++++++++++++++++---
include/drm/bridge/samsung-dsim.h | 1 +
2 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 08266303c261..d19a5c87b749 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -218,6 +218,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",
@@ -487,6 +489,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
.m_min = 64,
.m_max = 1023,
.min_freq = 1050,
+ .dynamic_dphy = 1,
};
static const struct samsung_dsim_driver_data *
@@ -698,13 +701,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
const unsigned int *reg_values = driver_data->reg_values;
u32 reg;
+ struct drm_display_mode *m = &dsi->mode;
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+ struct phy_configure_opts_mipi_dphy cfg;
+ int clk_prepare, lpx, clk_zero, clk_post, clk_trail;
+ int hs_exit, hs_prepare, hs_zero, hs_trail;
+ unsigned long long clock_in_hz = m->clock * 1000;
if (driver_data->has_freqband)
return;
+ /* The dynamic_phy has the ability to adjust PHY Timing settings */
+ if (driver_data->dynamic_dphy) {
+ phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg);
+
+ /*
+ * TODO:
+ * The tech reference manual for i.MX8M Mini/Nano/Plus
+ * doesn't state what the definition of the PHYTIMING
+ * bits are beyond their address and bit position.
+ * After reviewing NXP's downstream code, it appears
+ * that the various PHYTIMING registers take the number
+ * of cycles and use various dividers on them. This
+ * calculation does not result in an exact match to the
+ * downstream code, but it is very close, and it appears
+ * to sync at a variety of resolutions. If someone
+ * can get a more accurate mathematical equation needed
+ * for these registers, this should be updated.
+ */
+
+ lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz);
+ hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz);
+ clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz);
+ clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz);
+ clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz);
+ clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz);
+ hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz);
+ hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz);
+ hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz);
+ }
+
/* B D-PHY: D-PHY Master & Slave Analog Block control */
reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] |
reg_values[PHYCTRL_SLEW_UP];
+
samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg);
/*
@@ -712,7 +752,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
* T HS-EXIT: Time that the transmitter drives LP-11 following a HS
* burst
*/
- reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
+ if (driver_data->dynamic_dphy)
+ reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit);
+ else
+ reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
+
samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
/*
@@ -728,10 +772,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
* T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
* the last payload clock bit of a HS transmission burst
*/
- reg = reg_values[PHYTIMING_CLK_PREPARE] |
- reg_values[PHYTIMING_CLK_ZERO] |
- reg_values[PHYTIMING_CLK_POST] |
- reg_values[PHYTIMING_CLK_TRAIL];
+ if (driver_data->dynamic_dphy) {
+ reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) |
+ DSIM_PHYTIMING1_CLK_ZERO(clk_zero) |
+ DSIM_PHYTIMING1_CLK_POST(clk_post) |
+ DSIM_PHYTIMING1_CLK_TRAIL(clk_trail);
+ } else {
+ reg = reg_values[PHYTIMING_CLK_PREPARE] |
+ reg_values[PHYTIMING_CLK_ZERO] |
+ reg_values[PHYTIMING_CLK_POST] |
+ reg_values[PHYTIMING_CLK_TRAIL];
+ }
samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg);
@@ -744,8 +795,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
* T HS-TRAIL: Time that the transmitter drives the flipped differential
* state after last payload data bit of a HS transmission burst
*/
- reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] |
- reg_values[PHYTIMING_HS_TRAIL];
+
+ if (driver_data->dynamic_dphy) {
+ reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) |
+ DSIM_PHYTIMING2_HS_ZERO(hs_zero) |
+ DSIM_PHYTIMING2_HS_TRAIL(hs_trail);
+ } else {
+ reg = reg_values[PHYTIMING_HS_PREPARE] |
+ reg_values[PHYTIMING_HS_ZERO] |
+ reg_values[PHYTIMING_HS_TRAIL];
+ }
+
samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg);
}
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index a1a5b2b89a7a..76ea8a1720cc 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -62,6 +62,7 @@ struct samsung_dsim_driver_data {
const unsigned int *reg_values;
u16 m_min;
u16 m_max;
+ bool dynamic_dphy;
};
struct samsung_dsim_host_ops {
--
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]>
---
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 2be3b58624c3..bf4b33d2de76 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -405,6 +405,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 = {
@@ -418,6 +421,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 = {
@@ -429,6 +435,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 = {
@@ -441,6 +450,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 = {
@@ -453,6 +465,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 = {
@@ -469,6 +484,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 *
@@ -547,12 +565,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 ba5484de2b30..a1a5b2b89a7a 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
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]>
---
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
On Sun, May 7, 2023 at 3:25 AM Adam Ford <[email protected]> wrote:
>
> 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]>
Hi Adam,
Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
> The DPHY timings are currently hard coded. Since the input
> clock can be variable, the phy timings need to be variable
> too. Add an additional variable to the driver data to enable
> this feature to prevent breaking boards that don't support it.
>
> The phy_mipi_dphy_get_default_config function configures the
> DPHY timings in pico-seconds, and a small macro converts those
> timings into clock cycles based on the pixel clock rate.
>
This week I finally had some time to take a deeper look at this series
and test it on some of my systems.
This patch causes issues when the burst clock rate is fixed by
supplying the DT entry. Instead of describing the issue below, I'm
attaching the patch that makes things work on my system.
I would appreciate if you could test this one on your side. Feel free
to squash it into yours if you find it working properly.
Also I would almost bet that dynamic_dphy is working on the Exynos
boards with that fix added. So if anyone with access to those boards
would like to give it a shot, we may be able to get rid of the
hardcoded PHY parameters altogether, which would be a nice cleanup.
Regards,
Lucas
> 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: Michael Walle <[email protected]>
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 74 ++++++++++++++++++++++++---
> include/drm/bridge/samsung-dsim.h | 1 +
> 2 files changed, 68 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 08266303c261..d19a5c87b749 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -218,6 +218,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",
> @@ -487,6 +489,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> .m_min = 64,
> .m_max = 1023,
> .min_freq = 1050,
> + .dynamic_dphy = 1,
> };
>
> static const struct samsung_dsim_driver_data *
> @@ -698,13 +701,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> const unsigned int *reg_values = driver_data->reg_values;
> u32 reg;
> + struct drm_display_mode *m = &dsi->mode;
> + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> + struct phy_configure_opts_mipi_dphy cfg;
> + int clk_prepare, lpx, clk_zero, clk_post, clk_trail;
> + int hs_exit, hs_prepare, hs_zero, hs_trail;
> + unsigned long long clock_in_hz = m->clock * 1000;
>
> if (driver_data->has_freqband)
> return;
>
> + /* The dynamic_phy has the ability to adjust PHY Timing settings */
> + if (driver_data->dynamic_dphy) {
> + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg);
> +
> + /*
> + * TODO:
> + * The tech reference manual for i.MX8M Mini/Nano/Plus
> + * doesn't state what the definition of the PHYTIMING
> + * bits are beyond their address and bit position.
> + * After reviewing NXP's downstream code, it appears
> + * that the various PHYTIMING registers take the number
> + * of cycles and use various dividers on them. This
> + * calculation does not result in an exact match to the
> + * downstream code, but it is very close, and it appears
> + * to sync at a variety of resolutions. If someone
> + * can get a more accurate mathematical equation needed
> + * for these registers, this should be updated.
> + */
> +
> + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz);
> + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz);
> + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz);
> + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz);
> + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz);
> + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz);
> + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz);
> + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz);
> + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz);
> + }
> +
> /* B D-PHY: D-PHY Master & Slave Analog Block control */
> reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] |
> reg_values[PHYCTRL_SLEW_UP];
> +
> samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg);
>
> /*
> @@ -712,7 +752,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T HS-EXIT: Time that the transmitter drives LP-11 following a HS
> * burst
> */
> - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> + if (driver_data->dynamic_dphy)
> + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit);
> + else
> + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> +
> samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
>
> /*
> @@ -728,10 +772,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
> * the last payload clock bit of a HS transmission burst
> */
> - reg = reg_values[PHYTIMING_CLK_PREPARE] |
> - reg_values[PHYTIMING_CLK_ZERO] |
> - reg_values[PHYTIMING_CLK_POST] |
> - reg_values[PHYTIMING_CLK_TRAIL];
> + if (driver_data->dynamic_dphy) {
> + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) |
> + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) |
> + DSIM_PHYTIMING1_CLK_POST(clk_post) |
> + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail);
> + } else {
> + reg = reg_values[PHYTIMING_CLK_PREPARE] |
> + reg_values[PHYTIMING_CLK_ZERO] |
> + reg_values[PHYTIMING_CLK_POST] |
> + reg_values[PHYTIMING_CLK_TRAIL];
> + }
>
> samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg);
>
> @@ -744,8 +795,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> * T HS-TRAIL: Time that the transmitter drives the flipped differential
> * state after last payload data bit of a HS transmission burst
> */
> - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] |
> - reg_values[PHYTIMING_HS_TRAIL];
> +
> + if (driver_data->dynamic_dphy) {
> + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) |
> + DSIM_PHYTIMING2_HS_ZERO(hs_zero) |
> + DSIM_PHYTIMING2_HS_TRAIL(hs_trail);
> + } else {
> + reg = reg_values[PHYTIMING_HS_PREPARE] |
> + reg_values[PHYTIMING_HS_ZERO] |
> + reg_values[PHYTIMING_HS_TRAIL];
> + }
> +
> samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg);
> }
>
> diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> index a1a5b2b89a7a..76ea8a1720cc 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data {
> const unsigned int *reg_values;
> u16 m_min;
> u16 m_max;
> + bool dynamic_dphy;
> };
>
> struct samsung_dsim_host_ops {
On Fri, May 12, 2023 at 2:37 PM Lucas Stach <[email protected]> wrote:
>
> Hi Adam,
>
> Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
> > The DPHY timings are currently hard coded. Since the input
> > clock can be variable, the phy timings need to be variable
> > too. Add an additional variable to the driver data to enable
> > this feature to prevent breaking boards that don't support it.
> >
> > The phy_mipi_dphy_get_default_config function configures the
> > DPHY timings in pico-seconds, and a small macro converts those
> > timings into clock cycles based on the pixel clock rate.
> >
> This week I finally had some time to take a deeper look at this series
> and test it on some of my systems.
Thanks for testing this!
>
> This patch causes issues when the burst clock rate is fixed by
> supplying the DT entry. Instead of describing the issue below, I'm
> attaching the patch that makes things work on my system.
Oops, sorry about that.
>
> I would appreciate if you could test this one on your side. Feel free
> to squash it into yours if you find it working properly.
I reviewed your patch, and it looks like it makes a lot of sense.
If it works, I'll squash them together and add your name to the sign-off.
>
> Also I would almost bet that dynamic_dphy is working on the Exynos
> boards with that fix added. So if anyone with access to those boards
> would like to give it a shot, we may be able to get rid of the
> hardcoded PHY parameters altogether, which would be a nice cleanup.
I wondered the same thing, but I didn't want to create more work for
Marek S and since there was so much churn getting the original driver
ported, I thought it would be the safest thing to try to give the
imx8m m/n/p the features without breaking the Exynos.
Marek S - Do you want me to post this file without the extra checks to
see if it still works with Exynos?
adam
>
> Regards,
> Lucas
>
> > 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: Michael Walle <[email protected]>
> > ---
> > drivers/gpu/drm/bridge/samsung-dsim.c | 74 ++++++++++++++++++++++++---
> > include/drm/bridge/samsung-dsim.h | 1 +
> > 2 files changed, 68 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> > index 08266303c261..d19a5c87b749 100644
> > --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> > +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> > @@ -218,6 +218,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",
> > @@ -487,6 +489,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
> > .m_min = 64,
> > .m_max = 1023,
> > .min_freq = 1050,
> > + .dynamic_dphy = 1,
> > };
> >
> > static const struct samsung_dsim_driver_data *
> > @@ -698,13 +701,50 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> > const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> > const unsigned int *reg_values = driver_data->reg_values;
> > u32 reg;
> > + struct drm_display_mode *m = &dsi->mode;
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > + struct phy_configure_opts_mipi_dphy cfg;
> > + int clk_prepare, lpx, clk_zero, clk_post, clk_trail;
> > + int hs_exit, hs_prepare, hs_zero, hs_trail;
> > + unsigned long long clock_in_hz = m->clock * 1000;
> >
> > if (driver_data->has_freqband)
> > return;
> >
> > + /* The dynamic_phy has the ability to adjust PHY Timing settings */
> > + if (driver_data->dynamic_dphy) {
> > + phy_mipi_dphy_get_default_config(clock_in_hz, bpp, dsi->lanes, &cfg);
> > +
> > + /*
> > + * TODO:
> > + * The tech reference manual for i.MX8M Mini/Nano/Plus
> > + * doesn't state what the definition of the PHYTIMING
> > + * bits are beyond their address and bit position.
> > + * After reviewing NXP's downstream code, it appears
> > + * that the various PHYTIMING registers take the number
> > + * of cycles and use various dividers on them. This
> > + * calculation does not result in an exact match to the
> > + * downstream code, but it is very close, and it appears
> > + * to sync at a variety of resolutions. If someone
> > + * can get a more accurate mathematical equation needed
> > + * for these registers, this should be updated.
> > + */
> > +
> > + lpx = PS_TO_CYCLE(cfg.lpx, clock_in_hz);
> > + hs_exit = PS_TO_CYCLE(cfg.hs_exit, clock_in_hz);
> > + clk_prepare = PS_TO_CYCLE(cfg.clk_prepare, clock_in_hz);
> > + clk_zero = PS_TO_CYCLE(cfg.clk_zero, clock_in_hz);
> > + clk_post = PS_TO_CYCLE(cfg.clk_post, clock_in_hz);
> > + clk_trail = PS_TO_CYCLE(cfg.clk_trail, clock_in_hz);
> > + hs_prepare = PS_TO_CYCLE(cfg.hs_prepare, clock_in_hz);
> > + hs_zero = PS_TO_CYCLE(cfg.hs_zero, clock_in_hz);
> > + hs_trail = PS_TO_CYCLE(cfg.hs_trail, clock_in_hz);
> > + }
> > +
> > /* B D-PHY: D-PHY Master & Slave Analog Block control */
> > reg = reg_values[PHYCTRL_ULPS_EXIT] | reg_values[PHYCTRL_VREG_LP] |
> > reg_values[PHYCTRL_SLEW_UP];
> > +
> > samsung_dsim_write(dsi, DSIM_PHYCTRL_REG, reg);
> >
> > /*
> > @@ -712,7 +752,11 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> > * T HS-EXIT: Time that the transmitter drives LP-11 following a HS
> > * burst
> > */
> > - reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> > + if (driver_data->dynamic_dphy)
> > + reg = DSIM_PHYTIMING_LPX(lpx) | DSIM_PHYTIMING_HS_EXIT(hs_exit);
> > + else
> > + reg = reg_values[PHYTIMING_LPX] | reg_values[PHYTIMING_HS_EXIT];
> > +
> > samsung_dsim_write(dsi, DSIM_PHYTIMING_REG, reg);
> >
> > /*
> > @@ -728,10 +772,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> > * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
> > * the last payload clock bit of a HS transmission burst
> > */
> > - reg = reg_values[PHYTIMING_CLK_PREPARE] |
> > - reg_values[PHYTIMING_CLK_ZERO] |
> > - reg_values[PHYTIMING_CLK_POST] |
> > - reg_values[PHYTIMING_CLK_TRAIL];
> > + if (driver_data->dynamic_dphy) {
> > + reg = DSIM_PHYTIMING1_CLK_PREPARE(clk_prepare) |
> > + DSIM_PHYTIMING1_CLK_ZERO(clk_zero) |
> > + DSIM_PHYTIMING1_CLK_POST(clk_post) |
> > + DSIM_PHYTIMING1_CLK_TRAIL(clk_trail);
> > + } else {
> > + reg = reg_values[PHYTIMING_CLK_PREPARE] |
> > + reg_values[PHYTIMING_CLK_ZERO] |
> > + reg_values[PHYTIMING_CLK_POST] |
> > + reg_values[PHYTIMING_CLK_TRAIL];
> > + }
> >
> > samsung_dsim_write(dsi, DSIM_PHYTIMING1_REG, reg);
> >
> > @@ -744,8 +795,17 @@ static void samsung_dsim_set_phy_ctrl(struct samsung_dsim *dsi)
> > * T HS-TRAIL: Time that the transmitter drives the flipped differential
> > * state after last payload data bit of a HS transmission burst
> > */
> > - reg = reg_values[PHYTIMING_HS_PREPARE] | reg_values[PHYTIMING_HS_ZERO] |
> > - reg_values[PHYTIMING_HS_TRAIL];
> > +
> > + if (driver_data->dynamic_dphy) {
> > + reg = DSIM_PHYTIMING2_HS_PREPARE(hs_prepare) |
> > + DSIM_PHYTIMING2_HS_ZERO(hs_zero) |
> > + DSIM_PHYTIMING2_HS_TRAIL(hs_trail);
> > + } else {
> > + reg = reg_values[PHYTIMING_HS_PREPARE] |
> > + reg_values[PHYTIMING_HS_ZERO] |
> > + reg_values[PHYTIMING_HS_TRAIL];
> > + }
> > +
> > samsung_dsim_write(dsi, DSIM_PHYTIMING2_REG, reg);
> > }
> >
> > diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> > index a1a5b2b89a7a..76ea8a1720cc 100644
> > --- a/include/drm/bridge/samsung-dsim.h
> > +++ b/include/drm/bridge/samsung-dsim.h
> > @@ -62,6 +62,7 @@ struct samsung_dsim_driver_data {
> > const unsigned int *reg_values;
> > u16 m_min;
> > u16 m_max;
> > + bool dynamic_dphy;
> > };
> >
> > struct samsung_dsim_host_ops {
>
On 12.05.2023 22:00, Adam Ford wrote:
> On Fri, May 12, 2023 at 2:37 PM Lucas Stach <[email protected]> wrote:
>> Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
>>> The DPHY timings are currently hard coded. Since the input
>>> clock can be variable, the phy timings need to be variable
>>> too. Add an additional variable to the driver data to enable
>>> this feature to prevent breaking boards that don't support it.
>>>
>>> The phy_mipi_dphy_get_default_config function configures the
>>> DPHY timings in pico-seconds, and a small macro converts those
>>> timings into clock cycles based on the pixel clock rate.
>>>
>> This week I finally had some time to take a deeper look at this series
>> and test it on some of my systems.
> Thanks for testing this!
>> This patch causes issues when the burst clock rate is fixed by
>> supplying the DT entry. Instead of describing the issue below, I'm
>> attaching the patch that makes things work on my system.
> Oops, sorry about that.
>
>> I would appreciate if you could test this one on your side. Feel free
>> to squash it into yours if you find it working properly.
> I reviewed your patch, and it looks like it makes a lot of sense.
> If it works, I'll squash them together and add your name to the sign-off.
>
>> Also I would almost bet that dynamic_dphy is working on the Exynos
>> boards with that fix added. So if anyone with access to those boards
>> would like to give it a shot, we may be able to get rid of the
>> hardcoded PHY parameters altogether, which would be a nice cleanup.
> I wondered the same thing, but I didn't want to create more work for
> Marek S and since there was so much churn getting the original driver
> ported, I thought it would be the safest thing to try to give the
> imx8m m/n/p the features without breaking the Exynos.
>
> Marek S - Do you want me to post this file without the extra checks to
> see if it still works with Exynos?
Feel free to send me patches to test or just point to your
work-in-progress git repo.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Fri, May 12, 2023 at 4:02 PM Marek Szyprowski
<[email protected]> wrote:
>
> On 12.05.2023 22:00, Adam Ford wrote:
> > On Fri, May 12, 2023 at 2:37 PM Lucas Stach <[email protected]> wrote:
> >> Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
> >>> The DPHY timings are currently hard coded. Since the input
> >>> clock can be variable, the phy timings need to be variable
> >>> too. Add an additional variable to the driver data to enable
> >>> this feature to prevent breaking boards that don't support it.
> >>>
> >>> The phy_mipi_dphy_get_default_config function configures the
> >>> DPHY timings in pico-seconds, and a small macro converts those
> >>> timings into clock cycles based on the pixel clock rate.
> >>>
> >> This week I finally had some time to take a deeper look at this series
> >> and test it on some of my systems.
> > Thanks for testing this!
> >> This patch causes issues when the burst clock rate is fixed by
> >> supplying the DT entry. Instead of describing the issue below, I'm
> >> attaching the patch that makes things work on my system.
> > Oops, sorry about that.
> >
> >> I would appreciate if you could test this one on your side. Feel free
> >> to squash it into yours if you find it working properly.
> > I reviewed your patch, and it looks like it makes a lot of sense.
> > If it works, I'll squash them together and add your name to the sign-off.
That worked really well, I'll add it to my WIP directory since Marek S
said he'd test the other proposal of dropping the dynamic phy flag and
corresponding check in favor of pushing everyone to the same code.
> >
> >> Also I would almost bet that dynamic_dphy is working on the Exynos
> >> boards with that fix added. So if anyone with access to those boards
> >> would like to give it a shot, we may be able to get rid of the
> >> hardcoded PHY parameters altogether, which would be a nice cleanup.
> > I wondered the same thing, but I didn't want to create more work for
> > Marek S and since there was so much churn getting the original driver
> > ported, I thought it would be the safest thing to try to give the
> > imx8m m/n/p the features without breaking the Exynos.
> >
> > Marek S - Do you want me to post this file without the extra checks to
> > see if it still works with Exynos?
>
> Feel free to send me patches to test or just point to your
> work-in-progress git repo.
Thanks for testing this, Marek S. My work-in-progress branch is:
https://github.com/aford173/linux/tree/dsim-updates-wip
Depending on what you find will determine how I modify the next
revision of the code I push, so I very much appreciate your feedback.
Hopefully the suggestion from Lucas will work for your applications
and we can reduce some of the code complexity.
adam
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
On 13.05.2023 06:25, Adam Ford wrote:
> On Fri, May 12, 2023 at 4:02 PM Marek Szyprowski
> <[email protected]> wrote:
>> On 12.05.2023 22:00, Adam Ford wrote:
>>> On Fri, May 12, 2023 at 2:37 PM Lucas Stach <[email protected]> wrote:
>>>> Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
>>>>> The DPHY timings are currently hard coded. Since the input
>>>>> clock can be variable, the phy timings need to be variable
>>>>> too. Add an additional variable to the driver data to enable
>>>>> this feature to prevent breaking boards that don't support it.
>>>>>
>>>>> The phy_mipi_dphy_get_default_config function configures the
>>>>> DPHY timings in pico-seconds, and a small macro converts those
>>>>> timings into clock cycles based on the pixel clock rate.
>>>>>
>>>> This week I finally had some time to take a deeper look at this series
>>>> and test it on some of my systems.
>>> Thanks for testing this!
>>>> This patch causes issues when the burst clock rate is fixed by
>>>> supplying the DT entry. Instead of describing the issue below, I'm
>>>> attaching the patch that makes things work on my system.
>>> Oops, sorry about that.
>>>
>>>> I would appreciate if you could test this one on your side. Feel free
>>>> to squash it into yours if you find it working properly.
>>> I reviewed your patch, and it looks like it makes a lot of sense.
>>> If it works, I'll squash them together and add your name to the sign-off.
> That worked really well, I'll add it to my WIP directory since Marek S
> said he'd test the other proposal of dropping the dynamic phy flag and
> corresponding check in favor of pushing everyone to the same code.
>
>>>> Also I would almost bet that dynamic_dphy is working on the Exynos
>>>> boards with that fix added. So if anyone with access to those boards
>>>> would like to give it a shot, we may be able to get rid of the
>>>> hardcoded PHY parameters altogether, which would be a nice cleanup.
>>> I wondered the same thing, but I didn't want to create more work for
>>> Marek S and since there was so much churn getting the original driver
>>> ported, I thought it would be the safest thing to try to give the
>>> imx8m m/n/p the features without breaking the Exynos.
>>>
>>> Marek S - Do you want me to post this file without the extra checks to
>>> see if it still works with Exynos?
>> Feel free to send me patches to test or just point to your
>> work-in-progress git repo.
> Thanks for testing this, Marek S. My work-in-progress branch is:
>
> https://protect2.fireeye.com/v1/url?k=2eeb1ed9-4e098384-2eea9596-000babd9f1ba-9ad5c339e5ea6e4d&q=1&e=652be603-d622-4d0e-95d3-639656ab1af1&u=https%3A%2F%2Fgithub.com%2Faford173%2Flinux%2Ftree%2Fdsim-updates-wip
>
> Depending on what you find will determine how I modify the next
> revision of the code I push, so I very much appreciate your feedback.
> Hopefully the suggestion from Lucas will work for your applications
> and we can reduce some of the code complexity.
The above mentioned 'dsim-updates-wip' branch works fine on all my
Exynos based boards.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
On Mon, May 15, 2023 at 2:37 AM Marek Szyprowski
<[email protected]> wrote:
>
> On 13.05.2023 06:25, Adam Ford wrote:
> > On Fri, May 12, 2023 at 4:02 PM Marek Szyprowski
> > <[email protected]> wrote:
> >> On 12.05.2023 22:00, Adam Ford wrote:
> >>> On Fri, May 12, 2023 at 2:37 PM Lucas Stach <[email protected]> wrote:
> >>>> Am Samstag, dem 06.05.2023 um 14:24 -0500 schrieb Adam Ford:
> >>>>> The DPHY timings are currently hard coded. Since the input
> >>>>> clock can be variable, the phy timings need to be variable
> >>>>> too. Add an additional variable to the driver data to enable
> >>>>> this feature to prevent breaking boards that don't support it.
> >>>>>
> >>>>> The phy_mipi_dphy_get_default_config function configures the
> >>>>> DPHY timings in pico-seconds, and a small macro converts those
> >>>>> timings into clock cycles based on the pixel clock rate.
> >>>>>
> >>>> This week I finally had some time to take a deeper look at this series
> >>>> and test it on some of my systems.
> >>> Thanks for testing this!
> >>>> This patch causes issues when the burst clock rate is fixed by
> >>>> supplying the DT entry. Instead of describing the issue below, I'm
> >>>> attaching the patch that makes things work on my system.
> >>> Oops, sorry about that.
> >>>
> >>>> I would appreciate if you could test this one on your side. Feel free
> >>>> to squash it into yours if you find it working properly.
> >>> I reviewed your patch, and it looks like it makes a lot of sense.
> >>> If it works, I'll squash them together and add your name to the sign-off.
> > That worked really well, I'll add it to my WIP directory since Marek S
> > said he'd test the other proposal of dropping the dynamic phy flag and
> > corresponding check in favor of pushing everyone to the same code.
> >
> >>>> Also I would almost bet that dynamic_dphy is working on the Exynos
> >>>> boards with that fix added. So if anyone with access to those boards
> >>>> would like to give it a shot, we may be able to get rid of the
> >>>> hardcoded PHY parameters altogether, which would be a nice cleanup.
> >>> I wondered the same thing, but I didn't want to create more work for
> >>> Marek S and since there was so much churn getting the original driver
> >>> ported, I thought it would be the safest thing to try to give the
> >>> imx8m m/n/p the features without breaking the Exynos.
> >>>
> >>> Marek S - Do you want me to post this file without the extra checks to
> >>> see if it still works with Exynos?
> >> Feel free to send me patches to test or just point to your
> >> work-in-progress git repo.
> > Thanks for testing this, Marek S. My work-in-progress branch is:
> >
> > https://protect2.fireeye.com/v1/url?k=2eeb1ed9-4e098384-2eea9596-000babd9f1ba-9ad5c339e5ea6e4d&q=1&e=652be603-d622-4d0e-95d3-639656ab1af1&u=https%3A%2F%2Fgithub.com%2Faford173%2Flinux%2Ftree%2Fdsim-updates-wip
> >
> > Depending on what you find will determine how I modify the next
> > revision of the code I push, so I very much appreciate your feedback.
> > Hopefully the suggestion from Lucas will work for your applications
> > and we can reduce some of the code complexity.
>
> The above mentioned 'dsim-updates-wip' branch works fine on all my
> Exynos based boards.
Thank you for testing. I'll work on squashing some of the patches
together and eliminating some of the duplicative stuff so the end
result should be the same as what is in WIP and submit another
revision soon.
thanks!
adam
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>