linux-omap.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
@ 2012-02-15  5:50 mythripk
  2012-02-15  7:41 ` Tomi Valkeinen
  0 siblings, 1 reply; 6+ messages in thread
From: mythripk @ 2012-02-15  5:50 UTC (permalink / raw)
  To: linux-omap; +Cc: tomi.valkeinen, Mythri P K

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;
+
+	/*
 	 * 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;
+	pi->regmf = pi->regm2 * mf / refclk;
 
 	/*
 	 * Dcofreq should be set to 1 if required pixel clock
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
  2012-02-15  5:50 [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout mythripk
@ 2012-02-15  7:41 ` Tomi Valkeinen
  2012-02-15  9:25   ` K, Mythri P
  0 siblings, 1 reply; 6+ messages in thread
From: Tomi Valkeinen @ 2012-02-15  7:41 UTC (permalink / raw)
  To: mythripk; +Cc: linux-omap

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
  2012-02-15  7:41 ` Tomi Valkeinen
@ 2012-02-15  9:25   ` K, Mythri P
  2012-02-15 14:52     ` Tomi Valkeinen
  0 siblings, 1 reply; 6+ messages in thread
From: K, Mythri P @ 2012-02-15  9:25 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap

Hi,

On Wed, Feb 15, 2012 at 1:11 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 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.
Well this is just a copy of old code will change.
>
>> +
>> +     /*
>>        * 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:
Normally regm values are much higher compared to regm2 , regm2 is
normally 1 or 2.
also given that regm2 is a multiplier factor in calculating regm there
should not be any precision factor what do you think ?

Thanks and regards,
Mythri.
>
> pi->regm * refclk / pi->regm2
>
>  Tomi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
  2012-02-15  9:25   ` K, Mythri P
@ 2012-02-15 14:52     ` Tomi Valkeinen
  2012-02-16  6:42       ` K, Mythri P
  0 siblings, 1 reply; 6+ messages in thread
From: Tomi Valkeinen @ 2012-02-15 14:52 UTC (permalink / raw)
  To: K, Mythri P; +Cc: linux-omap

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

On Wed, 2012-02-15 at 14:55 +0530, K, Mythri P wrote:
> Hi,
> 
> On Wed, Feb 15, 2012 at 1:11 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > 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.
> Well this is just a copy of old code will change.

The multiplication and division with 100 is actually extra also, they
don't bring any precision here.

> >
> >> +
> >> +     /*
> >>        * 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:
> Normally regm values are much higher compared to regm2 , regm2 is
> normally 1 or 2.
> also given that regm2 is a multiplier factor in calculating regm there
> should not be any precision factor what do you think ?

Ok, I think it's fine.

 Tomi


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
  2012-02-15 14:52     ` Tomi Valkeinen
@ 2012-02-16  6:42       ` K, Mythri P
  2012-02-16  7:12         ` Tomi Valkeinen
  0 siblings, 1 reply; 6+ messages in thread
From: K, Mythri P @ 2012-02-16  6:42 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap

Hi Tomi,

On Wed, Feb 15, 2012 at 8:22 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2012-02-15 at 14:55 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Wed, Feb 15, 2012 at 1:11 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > 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.
>> Well this is just a copy of old code will change.
>
> The multiplication and division with 100 is actually extra also, they
> don't bring any precision here.
>
Well this is actually done to accommodate the pixel clock, For some
pixel clock like 25175 for VGA VESA
there is slight change which is detected by the analyzer so it is
added for that reason.
>> >
Thanks and regards,
Mythri.
>  Tomi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout
  2012-02-16  6:42       ` K, Mythri P
@ 2012-02-16  7:12         ` Tomi Valkeinen
  0 siblings, 0 replies; 6+ messages in thread
From: Tomi Valkeinen @ 2012-02-16  7:12 UTC (permalink / raw)
  To: K, Mythri P; +Cc: linux-omap

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

On Thu, 2012-02-16 at 12:12 +0530, K, Mythri P wrote:
> Hi Tomi,
> 
> On Wed, Feb 15, 2012 at 8:22 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2012-02-15 at 14:55 +0530, K, Mythri P wrote:
> >> Hi,
> >>
> >> On Wed, Feb 15, 2012 at 1:11 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > 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.
> >> Well this is just a copy of old code will change.
> >
> > The multiplication and division with 100 is actually extra also, they
> > don't bring any precision here.
> >
> Well this is actually done to accommodate the pixel clock, For some
> pixel clock like 25175 for VGA VESA
> there is slight change which is detected by the analyzer so it is
> added for that reason.

These two will always return the same value, so I don't think the
analyzer has detected that:

(phy * 100 * pi->regm2 / (refclk)) / 100

phy * pi->regm2 / refclk

 Tomi


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-16  7:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-15  5:50 [PATCH v2] OMAPDSS: HDMI: Add M2 divider while calculating clkout mythripk
2012-02-15  7:41 ` Tomi Valkeinen
2012-02-15  9:25   ` K, Mythri P
2012-02-15 14:52     ` Tomi Valkeinen
2012-02-16  6:42       ` K, Mythri P
2012-02-16  7:12         ` Tomi Valkeinen

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