All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <dominique.martinet@atmark-techno.com>
To: Adam Ford <aford173@gmail.com>
Cc: linux-phy@lists.infradead.org, linux-imx@nxp.com,
	festevam@gmail.com, frieder.schrempf@kontron.de,
	aford@beaconembedded.com, "Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Marco Felsch" <m.felsch@pengutronix.de>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	linux-kernel@vger.kernel.org,
	"Makoto Sato" <makoto.sato@atmark-techno.com>
Subject: Re: [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
Date: Wed, 28 Aug 2024 17:47:35 +0900	[thread overview]
Message-ID: <Zs7kJ6vCDbxVvxU1@atmark-techno.com> (raw)
In-Reply-To: <20240828024813.1353572-2-aford173@gmail.com>

Adam Ford wrote on Tue, Aug 27, 2024 at 09:48:02PM -0500:
> There is currently a look-up table for a variety of resolutions.
> Since the phy has the ability to dynamically calculate the values
> necessary to use the intger divider which should allow more
> resolutions without having to update the look-up-table.  If the
> integer calculator cannot get an exact frequency, it falls back
> to the look-up-table.  Because the LUT algorithm does some
> rounding, I did not remove integer entries from the LUT.

Thank you!

We're still running 5.10 so I backported the driver as of it's current
state first (that part works), unfortunately our 51.2MHz display does
not work with this.

After phy_clk_round_rate() not round the pixel clock to the table values
(otherwise we'd only get rounded values), and making phy_clk_set_rate()
pass the requested `rate` instead of using the next smaller cfg->pixclk,
the display no longer comes up.

It comes up with the values obtained for 50.4MHz (next closest value),
which also has an exact match so uses the integer divider this patch
computes instead of the table values, but not with the 51.2MHz it
requests...
I'm afraid at this point I don't know how to debug that further without
getting a scope out (I don't know if the soc isn't generating something
correct or if the display actually doesn't like the frequency it
requests?! the later could be checked by plugging it in to another PC
that might support that frequency...), and that is going to take quite a
while...

Hopefully Frieder will have more success with his displays?
It could also be very well due to some of the differences with our 5.10
tree, sorry about that.

>  static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
>  					  const struct phy_config *cfg)
>  {
> +	u32 desired_clock = cfg->pixclk * 5;

(I don't really understand where that `* 5` comes from, but I guess it's
expected? works for other display and neighbor frequency anyway...)

Thanks,
-- 
Dominique



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: Dominique Martinet <dominique.martinet@atmark-techno.com>
To: Adam Ford <aford173@gmail.com>
Cc: linux-phy@lists.infradead.org, linux-imx@nxp.com,
	festevam@gmail.com, frieder.schrempf@kontron.de,
	aford@beaconembedded.com, "Vinod Koul" <vkoul@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Marco Felsch" <m.felsch@pengutronix.de>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	linux-kernel@vger.kernel.org,
	"Makoto Sato" <makoto.sato@atmark-techno.com>
Subject: Re: [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider
Date: Wed, 28 Aug 2024 17:47:35 +0900	[thread overview]
Message-ID: <Zs7kJ6vCDbxVvxU1@atmark-techno.com> (raw)
In-Reply-To: <20240828024813.1353572-2-aford173@gmail.com>

Adam Ford wrote on Tue, Aug 27, 2024 at 09:48:02PM -0500:
> There is currently a look-up table for a variety of resolutions.
> Since the phy has the ability to dynamically calculate the values
> necessary to use the intger divider which should allow more
> resolutions without having to update the look-up-table.  If the
> integer calculator cannot get an exact frequency, it falls back
> to the look-up-table.  Because the LUT algorithm does some
> rounding, I did not remove integer entries from the LUT.

Thank you!

We're still running 5.10 so I backported the driver as of it's current
state first (that part works), unfortunately our 51.2MHz display does
not work with this.

After phy_clk_round_rate() not round the pixel clock to the table values
(otherwise we'd only get rounded values), and making phy_clk_set_rate()
pass the requested `rate` instead of using the next smaller cfg->pixclk,
the display no longer comes up.

It comes up with the values obtained for 50.4MHz (next closest value),
which also has an exact match so uses the integer divider this patch
computes instead of the table values, but not with the 51.2MHz it
requests...
I'm afraid at this point I don't know how to debug that further without
getting a scope out (I don't know if the soc isn't generating something
correct or if the display actually doesn't like the frequency it
requests?! the later could be checked by plugging it in to another PC
that might support that frequency...), and that is going to take quite a
while...

Hopefully Frieder will have more success with his displays?
It could also be very well due to some of the differences with our 5.10
tree, sorry about that.

>  static int fsl_samsung_hdmi_phy_configure(struct fsl_samsung_hdmi_phy *phy,
>  					  const struct phy_config *cfg)
>  {
> +	u32 desired_clock = cfg->pixclk * 5;

(I don't really understand where that `* 5` comes from, but I guess it's
expected? works for other display and neighbor frequency anyway...)

Thanks,
-- 
Dominique



  reply	other threads:[~2024-08-28  8:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-28  2:48 [RFC 1/2] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro Adam Ford
2024-08-28  2:48 ` Adam Ford
2024-08-28  2:48 ` [RFC 2/2] phy: freescale: fsl-samsung-hdmi: Support dynamic integer divider Adam Ford
2024-08-28  2:48   ` Adam Ford
2024-08-28  8:47   ` Dominique Martinet [this message]
2024-08-28  8:47     ` Dominique Martinet
2024-08-28 14:11     ` Adam Ford
2024-08-28 14:11       ` Adam Ford
2024-08-28 19:28       ` Adam Ford
2024-08-28 19:28         ` Adam Ford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zs7kJ6vCDbxVvxU1@atmark-techno.com \
    --to=dominique.martinet@atmark-techno.com \
    --cc=aford173@gmail.com \
    --cc=aford@beaconembedded.com \
    --cc=festevam@gmail.com \
    --cc=frieder.schrempf@kontron.de \
    --cc=kishon@kernel.org \
    --cc=l.stach@pengutronix.de \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=m.felsch@pengutronix.de \
    --cc=makoto.sato@atmark-techno.com \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.