[V8,0/7] drm: bridge: samsung-dsim: Support variable clocking

  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.
  

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
  

On 26/05/2023 09:22, Neil Armstrong wrote:
> Hi,
> 
> On Thu, 25 May 2023 22:05:52 -0500, Adam Ford wrote:
>> This series fixes the blanking pack size and the PMS calculation.  It then
>> adds support to allows the DSIM to dynamically DPHY clocks, and support
>> non-burst mode while allowing the removal of the hard-coded clock values
>> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
>> burst-clock device tree entry when burst-mode isn't supported by connected
>> devices like an HDMI brige.  In that event, the HS clock is set to the
>> value requested by the bridge chip.
>>
>> [...]
> 
> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
> 
> [1/7] drm: bridge: samsung-dsim: fix blanking packet size calculation
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=a617b33f7e513f25becf843bc97f8f1658c16337
> [2/7] drm: bridge: samsung-dsim: Fix PMS Calculator on imx8m[mnp]
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=54f1a83c72250b182fa7722b0c5f6eb5e769598d
> [3/7] drm: bridge: samsung-dsim: Fetch pll-clock-frequency automatically
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=33d8d14c83bf67aa0d262961a6fda9c40f3c1052
> [4/7] drm: bridge: samsung-dsim: Select GENERIC_PHY_MIPI_DPHY
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=171b3b1e0f8b8c894f2388e1cf765a56f831ee5e
> [5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=89691775f5735fca9dc40e119edcbb52a25b9612
> [6/7] drm: bridge: samsung-dsim: Support non-burst mode
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=bb0e13b9e223b218c9f242f8d340a332b4381042
> [7/7] dt-bindings: bridge: samsung-dsim: Make some flags optional
>        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=cfaf76d349837f695c8aa6d7077847fec4231fe5
> 

OK I made a bad manipulation, I applied patch 7 without review... I'll send a revert patch.

Neil
  

On Fri, May 26, 2023 at 2:24 AM Neil Armstrong
<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
  

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
  

On 26/05/2023 05.05, Adam Ford wrote:
> This series fixes the blanking pack size and the PMS calculation.  It then
> adds support to allows the DSIM to dynamically DPHY clocks, and support
> non-burst mode while allowing the removal of the hard-coded clock values
> for the PLL for imx8m mini/nano/plus, and it allows the removal of the
> burst-clock device tree entry when burst-mode isn't supported by connected
> devices like an HDMI brige.  In that event, the HS clock is set to the
> value requested by the bridge chip.
> 
> This has been tested on both an i.MX8M Nano and i.MX8M Plus, and should
> work on i.MX8M Mini as well. Marek Szyprowski has tested it on various
> Exynos boards.

Hi all

We're testing this on top of v6.4-rc4 on our imx8mp board, which has a
ti-sn65dsi86 DSI -> DisplayPort bridge. We do get an image at
1920x1200, but the monitor says it's only at 58Hz, and measuring on the
DSI signals does seem to confirm that the update frequency is about 57.7
or 57.8Hz (it's pretty hard to get a good measurement). It looks like
it's the lines that are too long, by a time that corresponds to about 80
pixels. But all the frontporch/backporch/hsync values look sane and
completely standard for that resolution.

Setting samsung,burst-clock-frequency explicitly to something large
enough or letting it be derived from the 154MHz pixel clock makes no
difference.

Any ideas?

Thanks,
Rasmus
  

On Wed, Jun 7, 2023 at 8:15 AM Rasmus Villemoes
<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
>
  

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

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
  

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
>
  

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