All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-26  3:05 ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Frieder Schrempf, devicetree, linux-kernel

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-26  3:05 ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, Laurent Pinchart, aford,
	Jernej Skrabec, Frieder Schrempf, devicetree, Rob Herring,
	Jagan Teki, Andrzej Hajda, Adam Ford, linux-kernel,
	Marek Szyprowski

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH V8 1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Lucas Stach, Adam Ford, Chen-Yu Tsai, Frieder Schrempf,
	Marek Szyprowski, Jagan Teki, Inki Dae, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel

From: Lucas Stach <l.stach@pengutronix.de>

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 <l.stach@pengutronix.de>
Signed-off-by: Adam Ford <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, Laurent Pinchart, aford,
	Frieder Schrempf, linux-kernel, devicetree, Rob Herring,
	Jagan Teki, Andrzej Hajda, Chen-Yu Tsai, Adam Ford,
	Marek Szyprowski

From: Lucas Stach <l.stach@pengutronix.de>

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 <l.stach@pengutronix.de>
Signed-off-by: Adam Ford <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Lucas Stach, Chen-Yu Tsai, Frieder Schrempf,
	Marek Szyprowski, Jagan Teki, Inki Dae, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, devicetree,
	linux-kernel

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 <aford173@gmail.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Andrzej Hajda, aford, Frieder Schrempf, Laurent Pinchart,
	Krzysztof Kozlowski, Marek Szyprowski, Marek Vasut, Robert Foss,
	Jernej Skrabec, Jagan Teki, Chen-Yu Tsai, devicetree,
	Conor Dooley, Jonas Karlman, Rob Herring, Adam Ford,
	Neil Armstrong, linux-kernel

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 <aford173@gmail.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Chen-Yu Tsai, Frieder Schrempf,
	Marek Szyprowski, Jagan Teki, Inki Dae, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel

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 <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # imx8mm-icore
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index ead922c3ce9f..307f1c20cfb9 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1719,11 +1719,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;
@@ -1736,19 +1736,27 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
 	u32 lane_polarities[5] = { 0 };
 	struct device_node *endpoint;
 	int i, nr_lanes, ret;
+	struct clk *pll_clk;
 
 	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_dbg(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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, Laurent Pinchart, aford,
	Frieder Schrempf, linux-kernel, devicetree, Rob Herring,
	Jagan Teki, Andrzej Hajda, Chen-Yu Tsai, Adam Ford,
	Marek Szyprowski

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 <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # imx8mm-icore
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index ead922c3ce9f..307f1c20cfb9 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1719,11 +1719,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;
@@ -1736,19 +1736,27 @@ static int samsung_dsim_parse_dt(struct samsung_dsim *dsi)
 	u32 lane_polarities[5] = { 0 };
 	struct device_node *endpoint;
 	int i, nr_lanes, ret;
+	struct clk *pll_clk;
 
 	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_dbg(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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Frieder Schrempf, Chen-Yu Tsai, Inki Dae,
	Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong,
	Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Marek Vasut, devicetree, linux-kernel

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 <aford173@gmail.com>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman,
	Laurent Pinchart, aford, Frieder Schrempf, linux-kernel,
	devicetree, Rob Herring, Jagan Teki, Andrzej Hajda, Chen-Yu Tsai,
	Adam Ford, Marek Szyprowski

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 <aford173@gmail.com>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
---
 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Andrzej Hajda, aford, Frieder Schrempf, Laurent Pinchart,
	Krzysztof Kozlowski, Marek Szyprowski, Robert Foss,
	Jernej Skrabec, Jagan Teki, Chen-Yu Tsai, devicetree,
	Conor Dooley, Jonas Karlman, Rob Herring, Adam Ford,
	Neil Armstrong, linux-kernel, Michael Walle

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 <aford173@gmail.com>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Michael Walle <michael@walle.cc>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Lucas Stach, Chen-Yu Tsai, Frieder Schrempf,
	Michael Walle, Marek Szyprowski, Jagan Teki, Inki Dae,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-kernel

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 <aford173@gmail.com>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Michael Walle <michael@walle.cc>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 6/7] drm: bridge: samsung-dsim: Support non-burst mode
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Chen-Yu Tsai, Frieder Schrempf,
	Marek Szyprowski, Jagan Teki, Inki Dae, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, devicetree,
	linux-kernel

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 <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 6/7] drm: bridge: samsung-dsim: Support non-burst mode
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman,
	Laurent Pinchart, aford, Frieder Schrempf, linux-kernel,
	devicetree, Rob Herring, Jagan Teki, Andrzej Hajda, Chen-Yu Tsai,
	Adam Ford, Marek Szyprowski

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 <aford173@gmail.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Reviewed-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # 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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  3:05   ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: Neil Armstrong, Conor Dooley, Robert Foss, Krzysztof Kozlowski,
	Jonas Karlman, Laurent Pinchart, aford, Jernej Skrabec,
	Frieder Schrempf, devicetree, Rob Herring, Jagan Teki,
	Andrzej Hajda, Adam Ford, linux-kernel, Marek Szyprowski

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 <aford173@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
@ 2023-05-26  3:05   ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26  3:05 UTC (permalink / raw)
  To: dri-devel
  Cc: aford, Adam Ford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frieder Schrempf,
	devicetree, linux-kernel

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 <aford173@gmail.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-05-26  3:05 ` Adam Ford
@ 2023-05-26  7:22   ` Neil Armstrong
  -1 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-26  7:22 UTC (permalink / raw)
  To: dri-devel, Adam Ford
  Cc: Marek Vasut, devicetree, Conor Dooley, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, Jernej Skrabec, Frieder Schrempf,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-26  7:22   ` Neil Armstrong
  0 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-26  7:22 UTC (permalink / raw)
  To: dri-devel, Adam Ford
  Cc: aford, Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Marek Vasut, Frieder Schrempf, devicetree,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-05-26  7:22   ` Neil Armstrong
@ 2023-05-26  7:24     ` Neil Armstrong
  -1 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-26  7:24 UTC (permalink / raw)
  To: dri-devel, Adam Ford
  Cc: aford, Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Marek Vasut, Frieder Schrempf, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-26  7:24     ` Neil Armstrong
  0 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-26  7:24 UTC (permalink / raw)
  To: dri-devel, Adam Ford
  Cc: Marek Vasut, devicetree, Conor Dooley, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, Jernej Skrabec, Frieder Schrempf,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-05-26  7:24     ` Neil Armstrong
@ 2023-05-26 14:04       ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26 14:04 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Frieder Schrempf,
	devicetree, linux-kernel

On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
<neil.armstrong@linaro.org> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-26 14:04       ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26 14:04 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Marek Vasut, devicetree, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
<neil.armstrong@linaro.org> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
  2023-05-26  3:05   ` Adam Ford
@ 2023-05-26 18:19     ` Conor Dooley
  -1 siblings, 0 replies; 44+ messages in thread
From: Conor Dooley @ 2023-05-26 18:19 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frieder Schrempf,
	devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]

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 <aford173@gmail.com>
> ---
>  .../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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
@ 2023-05-26 18:19     ` Conor Dooley
  0 siblings, 0 replies; 44+ messages in thread
From: Conor Dooley @ 2023-05-26 18:19 UTC (permalink / raw)
  To: Adam Ford
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]

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 <aford173@gmail.com>
> ---
>  .../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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
  2023-05-26 18:19     ` Conor Dooley
@ 2023-05-26 19:24       ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26 19:24 UTC (permalink / raw)
  To: Conor Dooley
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frieder Schrempf,
	devicetree, linux-kernel

On Fri, May 26, 2023 at 1:19 PM Conor Dooley <conor@kernel.org> 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 <aford173@gmail.com>
> > ---
> >  .../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
> >

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
@ 2023-05-26 19:24       ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-05-26 19:24 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On Fri, May 26, 2023 at 1:19 PM Conor Dooley <conor@kernel.org> 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 <aford173@gmail.com>
> > ---
> >  .../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
> >

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
  2023-05-26 19:24       ` Adam Ford
@ 2023-05-26 19:30         ` Conor Dooley
  -1 siblings, 0 replies; 44+ messages in thread
From: Conor Dooley @ 2023-05-26 19:30 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frieder Schrempf,
	devicetree, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]

On Fri, May 26, 2023 at 02:24:21PM -0500, Adam Ford wrote:
> On Fri, May 26, 2023 at 1:19 PM Conor Dooley <conor@kernel.org> 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 <conor.dooley@microchip.com>

> 
> 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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
@ 2023-05-26 19:30         ` Conor Dooley
  0 siblings, 0 replies; 44+ messages in thread
From: Conor Dooley @ 2023-05-26 19:30 UTC (permalink / raw)
  To: Adam Ford
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]

On Fri, May 26, 2023 at 02:24:21PM -0500, Adam Ford wrote:
> On Fri, May 26, 2023 at 1:19 PM Conor Dooley <conor@kernel.org> 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 <conor.dooley@microchip.com>

> 
> 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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
  2023-05-26 19:30         ` Conor Dooley
@ 2023-05-30  7:48           ` Krzysztof Kozlowski
  -1 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-30  7:48 UTC (permalink / raw)
  To: Conor Dooley, Adam Ford
  Cc: Neil Armstrong, Conor Dooley, Jernej Skrabec, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

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 <conor@kernel.org> 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 <conor.dooley@microchip.com>
> 
>>
>> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
@ 2023-05-30  7:48           ` Krzysztof Kozlowski
  0 siblings, 0 replies; 44+ messages in thread
From: Krzysztof Kozlowski @ 2023-05-30  7:48 UTC (permalink / raw)
  To: Conor Dooley, Adam Ford
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Frieder Schrempf,
	devicetree, linux-kernel

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 <conor@kernel.org> 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 <conor.dooley@microchip.com>
> 
>>
>> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-05-26 14:04       ` Adam Ford
@ 2023-05-30  8:01         ` Neil Armstrong
  -1 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-30  8:01 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Frieder Schrempf,
	devicetree, linux-kernel

On 26/05/2023 16:04, Adam Ford wrote:
> On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
> <neil.armstrong@linaro.org> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-05-30  8:01         ` Neil Armstrong
  0 siblings, 0 replies; 44+ messages in thread
From: Neil Armstrong @ 2023-05-30  8:01 UTC (permalink / raw)
  To: Adam Ford
  Cc: Marek Vasut, devicetree, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On 26/05/2023 16:04, Adam Ford wrote:
> On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
> <neil.armstrong@linaro.org> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-05-26  3:05 ` Adam Ford
@ 2023-06-07 13:15   ` Rasmus Villemoes
  -1 siblings, 0 replies; 44+ messages in thread
From: Rasmus Villemoes @ 2023-06-07 13:15 UTC (permalink / raw)
  To: Adam Ford, dri-devel
  Cc: aford, Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
	Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
	Jernej Skrabec, David Airlie, Daniel Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Frieder Schrempf,
	devicetree, linux-kernel

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-07 13:15   ` Rasmus Villemoes
  0 siblings, 0 replies; 44+ messages in thread
From: Rasmus Villemoes @ 2023-06-07 13:15 UTC (permalink / raw)
  To: Adam Ford, dri-devel
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Robert Foss,
	Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, Jernej Skrabec, Frieder Schrempf,
	devicetree, Rob Herring, Jagan Teki, Andrzej Hajda,
	Marek Szyprowski

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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-06-07 13:15   ` Rasmus Villemoes
@ 2023-06-07 13:27     ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-07 13:27 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Frieder Schrempf, devicetree, linux-kernel

On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> 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
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-07 13:27     ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-07 13:27 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> 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
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-06-07 13:27     ` Adam Ford
@ 2023-06-07 14:23       ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-07 14:23 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Frieder Schrempf, devicetree, linux-kernel

On Wed, Jun 7, 2023 at 8:27 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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
> >

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-07 14:23       ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-07 14:23 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On Wed, Jun 7, 2023 at 8:27 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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
> >

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-06-07 13:27     ` Adam Ford
@ 2023-06-08 11:40       ` Rasmus Villemoes
  -1 siblings, 0 replies; 44+ messages in thread
From: Rasmus Villemoes @ 2023-06-08 11:40 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Frieder Schrempf, devicetree, linux-kernel

On 07/06/2023 15.27, Adam Ford wrote:
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-08 11:40       ` Rasmus Villemoes
  0 siblings, 0 replies; 44+ messages in thread
From: Rasmus Villemoes @ 2023-06-08 11:40 UTC (permalink / raw)
  To: Adam Ford
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On 07/06/2023 15.27, Adam Ford wrote:
> On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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


^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-06-08 11:40       ` Rasmus Villemoes
@ 2023-06-08 12:30         ` Adam Ford
  -1 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-08 12:30 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: dri-devel, aford, Inki Dae, Jagan Teki, Marek Szyprowski,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Marek Vasut,
	Frieder Schrempf, devicetree, linux-kernel

On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 07/06/2023 15.27, Adam Ford wrote:
> > On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> 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
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-08 12:30         ` Adam Ford
  0 siblings, 0 replies; 44+ messages in thread
From: Adam Ford @ 2023-06-08 12:30 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 07/06/2023 15.27, Adam Ford wrote:
> > On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> 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
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
  2023-06-08 12:30         ` Adam Ford
@ 2023-06-08 12:52           ` Lucas Stach
  -1 siblings, 0 replies; 44+ messages in thread
From: Lucas Stach @ 2023-06-08 12:52 UTC (permalink / raw)
  To: Adam Ford, Rasmus Villemoes
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Jernej Skrabec,
	Robert Foss, Krzysztof Kozlowski, Jonas Karlman, linux-kernel,
	Laurent Pinchart, aford, dri-devel, Frieder Schrempf, devicetree,
	Rob Herring, Jagan Teki, Andrzej Hajda, Marek Szyprowski

Am Donnerstag, dem 08.06.2023 um 07:30 -0500 schrieb Adam Ford:
> On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking
@ 2023-06-08 12:52           ` Lucas Stach
  0 siblings, 0 replies; 44+ messages in thread
From: Lucas Stach @ 2023-06-08 12:52 UTC (permalink / raw)
  To: Adam Ford, Rasmus Villemoes
  Cc: Marek Vasut, Neil Armstrong, Conor Dooley, Robert Foss,
	Frieder Schrempf, Jonas Karlman, Andrzej Hajda, dri-devel,
	linux-kernel, Jernej Skrabec, aford, devicetree, Rob Herring,
	Laurent Pinchart, Krzysztof Kozlowski, Marek Szyprowski,
	Jagan Teki

Am Donnerstag, dem 08.06.2023 um 07:30 -0500 schrieb Adam Ford:
> On Thu, Jun 8, 2023 at 6:40 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2023-06-08 12:53 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-26  3:05 [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking Adam Ford
2023-05-26  3:05 ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp] Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 6/7] drm: bridge: samsung-dsim: Support non-burst mode Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26  3:05 ` [PATCH V8 7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional Adam Ford
2023-05-26  3:05   ` Adam Ford
2023-05-26 18:19   ` Conor Dooley
2023-05-26 18:19     ` Conor Dooley
2023-05-26 19:24     ` Adam Ford
2023-05-26 19:24       ` Adam Ford
2023-05-26 19:30       ` Conor Dooley
2023-05-26 19:30         ` Conor Dooley
2023-05-30  7:48         ` Krzysztof Kozlowski
2023-05-30  7:48           ` Krzysztof Kozlowski
2023-05-26  7:22 ` [PATCH V8 0/7] drm: bridge: samsung-dsim: Support variable clocking Neil Armstrong
2023-05-26  7:22   ` Neil Armstrong
2023-05-26  7:24   ` Neil Armstrong
2023-05-26  7:24     ` Neil Armstrong
2023-05-26 14:04     ` Adam Ford
2023-05-26 14:04       ` Adam Ford
2023-05-30  8:01       ` Neil Armstrong
2023-05-30  8:01         ` Neil Armstrong
2023-06-07 13:15 ` Rasmus Villemoes
2023-06-07 13:15   ` Rasmus Villemoes
2023-06-07 13:27   ` Adam Ford
2023-06-07 13:27     ` Adam Ford
2023-06-07 14:23     ` Adam Ford
2023-06-07 14:23       ` Adam Ford
2023-06-08 11:40     ` Rasmus Villemoes
2023-06-08 11:40       ` Rasmus Villemoes
2023-06-08 12:30       ` Adam Ford
2023-06-08 12:30         ` Adam Ford
2023-06-08 12:52         ` Lucas Stach
2023-06-08 12:52           ` Lucas Stach

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.