LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Chris Morgan <macroalpha82@gmail.com>
Cc: abel.vesa@nxp.com, festevam@gmail.com, heiko@sntech.de,
	kernel@pengutronix.de, lee.jones@linaro.org, lenb@kernel.org,
	linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org, linux-imx@nxp.com,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	mturquette@baylibre.com, rafael.j.wysocki@intel.com,
	rjw@rjwysocki.net, s.hauer@pengutronix.de, sboyd@kernel.org,
	shawnguo@kernel.org, zhangqing@rock-chips.com,
	Chris Morgan <macromorgan@hotmail.com>
Subject: Re: [PATCH v4 1/4] clk: fractional-divider: Export approximation algorithm to the CCF users
Date: Tue, 7 Sep 2021 21:06:10 +0300	[thread overview]
Message-ID: <YTeqEilet1p4vTAU@smile.fi.intel.com> (raw)
In-Reply-To: <YTenPMOOepGo2ZU8@smile.fi.intel.com>

On Tue, Sep 07, 2021 at 08:54:04PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 07, 2021 at 10:44:00AM -0500, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Unfortunately, I can confirm this breaks the DSI panel on the Rockchip
> > PX30 (and possibly other SoCs). Tested on my Odroid Go Advance. When
> > I revert 4e7cf74fa3b2 "clk: fractional-divider: Export approximation
> > algorithm to the CCF users" and 928f9e268611 "clk: fractional-divider:
> > Hide clk_fractional_divider_ops from wide audience" the panel begins
> > working again as expected on the master branch.
> > 
> > It looks like an assumption is made in the vop_crtc_mode_fixup()
> > function in the rockchip_drm_vop.c that gets broken with this change.
> > Specifically, the function says in the comments "When DRM gives us a
> > mode, we should add 999 Hz to it.". I believe this is no longer true
> > after this clk change, and when I remove the + 999 from the function
> > the DSI panel works again. Note that I do not know the implications
> > of removing this 999 aside from that it fixes the DSI panel on my
> > PX30 after this change, so I don't know if it's a positive change
> > or not.
> > 
> > Thank you.
> 
> Thank you for the report!
> 
> I'll check this. Perhaps Heiko can help with testing as well on his side.

On the first glance the mentioned patch may not be the culprit because it does
not change the functional behaviour (if I'm not mistaken). What really changes
it is the additional flag that removes the left-shift of the rate in the
calculations.

To me sounds like you found a proper solution to the issue and that +999 is
a hack against the (blindly?) copied part of the algorithm used in fractional
divider. Have you read the top comment in clk-fractional-divider.c? It should
explain how it works after my series.

In any case I'm not going to come to any conclusions right now and also want
to hear from people who have better understanding of this hardware.

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2021-09-07 18:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-12 17:00 Andy Shevchenko
2021-08-12 17:00 ` [PATCH v4 2/4] clk: fractional-divider: Hide clk_fractional_divider_ops from wide audience Andy Shevchenko
2021-08-12 17:00 ` [PATCH v4 3/4] clk: fractional-divider: Introduce POWER_OF_TWO_PS flag Andy Shevchenko
2021-08-12 17:00 ` [PATCH v4 4/4] clk: fractional-divider: Document the arithmetics used behind the code Andy Shevchenko
     [not found] ` <162879819529.19113.6409882476721828944@swboyd.mtv.corp.google.com>
2021-08-13  9:43   ` [PATCH v4 1/4] clk: fractional-divider: Export approximation algorithm to the CCF users Andy Shevchenko
2021-08-17 12:45     ` Andy Shevchenko
2021-09-07 15:44 ` Chris Morgan
2021-09-07 17:54   ` Andy Shevchenko
2021-09-07 18:06     ` Andy Shevchenko [this message]
     [not found]       ` <SN6PR06MB534203466EA3D4E22858479FA5D49@SN6PR06MB5342.namprd06.prod.outlook.com>
2021-09-08 10:52         ` Andy Shevchenko

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=YTeqEilet1p4vTAU@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=abel.vesa@nxp.com \
    --cc=festevam@gmail.com \
    --cc=heiko@sntech.de \
    --cc=kernel@pengutronix.de \
    --cc=lee.jones@linaro.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=macroalpha82@gmail.com \
    --cc=macromorgan@hotmail.com \
    --cc=mturquette@baylibre.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=s.hauer@pengutronix.de \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=zhangqing@rock-chips.com \
    --subject='Re: [PATCH v4 1/4] clk: fractional-divider: Export approximation algorithm to the CCF users' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).