Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On Wed, 2012-02-15 at 11:20 +0530, mythripk@ti.com wrote:
> From: Mythri P K <mythripk@ti.com>
> 
> Add M2 divider in the equation to calculate regm and regmf.
> Formula for calculating:
> Output clock on digital core domain:
> 	CLKOUT = (M / (N+1))*CLKINP*(1/M2)
> Internal oscillator output clock on internal LDO domain:
> 	CLKDCOLDO = (M / (N+1))*CLKINP
> The current code when allows variable M2 values as input
> ignores using M2 divider values in calculation of regm and regmf.
> so fix it by using M2 in calculation although the default value for
> M2 is 1.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> ---
>  drivers/video/omap2/dss/hdmi.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 92a6679..9185630 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -256,24 +256,24 @@ static void hdmi_compute_pll(struct omap_dss_device *dssdev, int phy,
>  
>  	refclk = clkin / pi->regn;
>  
> -	/*
> -	 * multiplier is pixel_clk/ref_clk
> -	 * Multiplying by 100 to avoid fractional part removal
> -	 */
> -	pi->regm = (phy * 100 / (refclk)) / 100;
> -
>  	if (dssdev->clocks.hdmi.regm2 == 0)
>  		pi->regm2 = HDMI_DEFAULT_REGM2;
>  	else
>  		pi->regm2 = dssdev->clocks.hdmi.regm2;
>  
>  	/*
> +	 * multiplier is pixel_clk/ref_clk
> +	 * Multiplying by 100 to avoid fractional part removal
> +	 */
> +	pi->regm = (phy * 100 * pi->regm2 / (refclk)) / 100;

No need for parenthesis around refclk.

> +
> +	/*
>  	 * fractional multiplier is remainder of the difference between
>  	 * multiplier and actual phy(required pixel clock thus should be
>  	 * multiplied by 2^18(262144) divided by the reference clock
>  	 */
> -	mf = (phy - pi->regm * refclk) * 262144;
> -	pi->regmf = mf / (refclk);
> +	mf = (phy - pi->regm / pi->regm2 * refclk) * 262144;

What kind of values does regm have? If the regm2 is something else than
1, and regm is relatively small value, it's quite easy to lose precision
there. Would it be better to have:

pi->regm * refclk / pi->regm2

 Tomi
 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]