[Intel-gfx] [PATCH 6/7] drm/i915/cnl: Write dco_fraction calculation as spec.

[Rodrigo Vivi]

On Tue, Nov 14, 2017 at 09:29:57PM +0000, Rodrigo Vivi wrote:
> On Tue, Nov 14, 2017 at 08:46:38PM +0000, Ville Syrjälä wrote:
> > On Tue, Nov 14, 2017 at 10:22:27PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 14, 2017 at 11:47:58AM -0800, Rodrigo Vivi wrote:
> > > > I confess I never fully understood that previous calculation,
> > > > so maybe this is cannot be called a "fix". But let's follow
> > > > the math that is written on Spec so we have get more confident
> > > > this is what hardware expect.
> > > >
> > > > Cc: Mika Kahola <mika.kahola at intel.com>
> > > > Cc: Manasi Navare <manasi.d.navare at intel.com>
> > > > Cc: James Ausmus <james.ausmus at intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > index bd608f7f2399..ee690d2f6e54 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > @@ -2190,8 +2190,8 @@ static void cnl_wrpll_params_populate(struct skl_wrpll_params *params,
> > > >  	params->qdiv_mode = (qdiv == 1) ? 0 : 1;
> > > >
> > > >  	params->dco_integer = div_u64(dco_freq, ref_freq);
> > > > -	params->dco_fraction = div_u64((div_u64((uint64_t)dco_freq<<15, (uint64_t)ref_freq) -
> > > > -					((uint64_t)params->dco_integer<<15)) * 0x8000, 0x8000);
> > > > +	params->dco_fraction = (DIV_ROUND_UP_ULL(dco_freq, ref_freq) -
> > > > +				params->dco_integer) * (1 << 15);
> > >
> > > Is dco_freq in khz here? Or hz?
> > >
> > > If khz, the following should work... I think:
> > >
>
> > > unsigned int dco = div_u64((u64)dco_freq << 15, ref_freq * 1000);
> >
> > Actually w/o the *1000 I suppose, for khz.
>
> On python it works apparently:
> * (7998750 << 15)/24000
> 10920960 #"dco"
> * ((7998750 << 15)/24000)>>15
> 333 #int
> * ((7998750 << 15)/24000)&0x7ffff
> 435200 #frac
>
> while on our code we get
> dco = 4584
> so int = 0
> and frac = 4584...
>
> div_u64 not working in float?

oh! u32 << 15 was the issue...

So, with this code:

u64 dco = dco_freq;
dco = div_u64(dco << 15, ref_freq);
params->dco_integer = dco >> 15;
params->dco_fraction = dco & 0x7fff;

we have int 333 as expected and frac 92116 as the orig code.
So it seems the orig code was better after all. Although I'd
still want to clean it up.

But what bothers me now is that python gives a different frac
and also calculator shows this division as 333.28125

nothing seems to make us to get this 28125...

:/

but now that I know how it is all my attempts here lead me to 92116
and it works...

Thanks,
Rodrigo.

>
> also this shows me that while this python gives us the frac 435200
> my current code that follows spec gives frac 32768
> and previous code was giving frac 9216
>
> :/
>
> more thoughts?
>
> >
> > > dco_int = dco >> 15;
> > > dco_frac = dco & 0x7fff;
> > >
> > > >  }
> > > >
> > > >  static bool
> > > > --
> > > > 2.13.6
> > > >
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC