linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: AngeloGioacchino Del Regno  <angelogioacchino.delregno@somainline.org>
To: linux-arm-msm@vger.kernel.org
Cc: konrad.dybcio@somainline.org, marijn.suijten@somainline.org,
	martin.botka@somainline.org, phone-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org, robdclark@gmail.com,
	sean@poorly.run, airlied@linux.ie, daniel@ffwll.ch,
	dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@somainline.org>
Subject: [PATCH 3/5] drm/msm/dsi_pll_10nm: Fix bad VCO rate calculation and prescaler
Date: Sat,  9 Jan 2021 14:51:10 +0100	[thread overview]
Message-ID: <20210109135112.147759-4-angelogioacchino.delregno@somainline.org> (raw)
In-Reply-To: <20210109135112.147759-1-angelogioacchino.delregno@somainline.org>

The VCO rate was being miscalculated due to a big overlook during
the process of porting this driver from downstream to upstream:
here we are really recalculating the rate of the VCO by reading
the appropriate registers and returning a real frequency, while
downstream the driver was doing something entirely different.

In our case here, the recalculated rate was wrong, as it was then
given back to the set_rate function, which was erroneously doing
a division on the fractional value, based on the prescaler being
either enabled or disabled: this was actually producing a bug for
which the final VCO rate was being doubled, causing very obvious
issues when trying to drive a DSI panel because the actual divider
value was multiplied by two!

To make things work properly, remove the multiplication of the
reference clock by two from function dsi_pll_calc_dec_frac and
account for the prescaler enablement in the vco_recalc_rate (if
the prescaler is enabled, then the hardware will divide the rate
by two).

This will make the vco_recalc_rate function to pass the right
frequency to the (clock framework) set_rate function when called,
which will - in turn - program the right values in both the
DECIMAL_DIV_START_1 and the FRAC_DIV_START_{LOW/MID/HIGH}_1
registers, finally making the PLL to output the right clock.

Also, while at it, remove the prescaler TODO by also adding the
possibility of disabling the prescaler on the PLL (it is in the
PLL_ANALOG_CONTROLS_ONE register).
Of course, both prescaler-ON and OFF cases were tested.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>
---
 drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
index 8b66e852eb36..5be562dfbf06 100644
--- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
+++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_10nm.c
@@ -165,11 +165,7 @@ static void dsi_pll_calc_dec_frac(struct dsi_pll_10nm *pll)
 
 	pll_freq = pll->vco_current_rate;
 
-	if (config->disable_prescaler)
-		divider = fref;
-	else
-		divider = fref * 2;
-
+	divider = fref;
 	multiplier = 1 << config->frac_bits;
 	dec_multiple = div_u64(pll_freq * multiplier, divider);
 	dec = div_u64_rem(dec_multiple, multiplier, &frac);
@@ -266,9 +262,11 @@ static void dsi_pll_ssc_commit(struct dsi_pll_10nm *pll)
 
 static void dsi_pll_config_hzindep_reg(struct dsi_pll_10nm *pll)
 {
+	struct dsi_pll_config *config = &pll->pll_configuration;
 	void __iomem *base = pll->mmio;
+	u32 val = config->disable_prescaler ? 0x0 : 0x80;
 
-	pll_write(base + REG_DSI_10nm_PHY_PLL_ANALOG_CONTROLS_ONE, 0x80);
+	pll_write(base + REG_DSI_10nm_PHY_PLL_ANALOG_CONTROLS_ONE, val);
 	pll_write(base + REG_DSI_10nm_PHY_PLL_ANALOG_CONTROLS_TWO, 0x03);
 	pll_write(base + REG_DSI_10nm_PHY_PLL_ANALOG_CONTROLS_THREE, 0x00);
 	pll_write(base + REG_DSI_10nm_PHY_PLL_DSM_DIVIDER, 0x00);
@@ -499,17 +497,15 @@ static unsigned long dsi_pll_10nm_vco_recalc_rate(struct clk_hw *hw,
 	frac |= ((pll_read(base + REG_DSI_10nm_PHY_PLL_FRAC_DIV_START_HIGH_1) &
 		  0x3) << 16);
 
-	/*
-	 * TODO:
-	 *	1. Assumes prescaler is disabled
-	 */
 	multiplier = 1 << config->frac_bits;
-	pll_freq = dec * (ref_clk * 2);
-	tmp64 = (ref_clk * 2 * frac);
+	pll_freq = dec * ref_clk;
+	tmp64 = ref_clk * frac;
 	pll_freq += div_u64(tmp64, multiplier);
-
 	vco_rate = pll_freq;
 
+	if (config->disable_prescaler)
+		vco_rate = div_u64(vco_rate, 2);
+
 	DBG("DSI PLL%d returning vco rate = %lu, dec = %x, frac = %x",
 	    pll_10nm->id, (unsigned long)vco_rate, dec, frac);
 
-- 
2.29.2


  parent reply	other threads:[~2021-01-09 13:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-09 13:51 [PATCH 0/5] Clock fixes for DSI 10nm PLL AngeloGioacchino Del Regno
2021-01-09 13:51 ` [PATCH 1/5] drm/msm/dsi_pll_10nm: Fix dividing the same numbers twice AngeloGioacchino Del Regno
2021-01-09 13:51 ` [PATCH 2/5] drm/msm/dsi_pll_10nm: Solve TODO for multiplier frac_bits assignment AngeloGioacchino Del Regno
2021-01-09 13:51 ` AngeloGioacchino Del Regno [this message]
2021-01-31 19:50   ` [PATCH 3/5] drm/msm/dsi_pll_10nm: Fix bad VCO rate calculation and prescaler Rob Clark
2021-02-01 10:11     ` AngeloGioacchino Del Regno
2021-02-01 15:47       ` Rob Clark
2021-02-01 17:05         ` Rob Clark
2021-02-01 17:18           ` Rob Clark
2021-02-01 17:31             ` Rob Clark
2021-02-02 14:32               ` AngeloGioacchino Del Regno
2021-02-02 18:45                 ` Rob Clark
2021-02-02 18:46                   ` AngeloGioacchino Del Regno
2021-02-02 19:08                     ` Rob Clark
2021-02-02 21:35                       ` Rob Clark
2021-01-09 13:51 ` [PATCH 4/5] drm/msm/dsi_pll_10nm: Fix variable usage for pll_lockdet_rate AngeloGioacchino Del Regno
2021-01-09 13:51 ` [PATCH 5/5] drm/msm/dsi_pll_10nm: Convert pr_err prints to DRM_DEV_ERROR AngeloGioacchino Del Regno

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=20210109135112.147759-4-angelogioacchino.delregno@somainline.org \
    --to=angelogioacchino.delregno@somainline.org \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=martin.botka@somainline.org \
    --cc=phone-devel@vger.kernel.org \
    --cc=robdclark@gmail.com \
    --cc=sean@poorly.run \
    /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 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).