[Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation changes for glk

[Chauhan, Madhav]

> -----Original Message-----
> From: Chauhan, Madhav
> Sent: Friday, March 17, 2017 7:11 PM
> To: Nikula, Jani <jani.nikula at intel.com>; Ander Conselvan De Oliveira
> <conselvan2 at gmail.com>; intel-gfx at lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation changes for
> glk
> 
> > -----Original Message-----
> > From: Nikula, Jani
> > Sent: Thursday, March 16, 2017 7:00 PM
> > To: Ander Conselvan De Oliveira <conselvan2 at gmail.com>; Chauhan,
> > Madhav <madhav.chauhan at intel.com>; intel-gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/glk: CDCLK calculation
> > changes for glk
> >
> > On Thu, 16 Mar 2017, Ander Conselvan De Oliveira
> > <conselvan2 at gmail.com> wrote:
> > > On Thu, 2017-03-16 at 15:10 +0200, Jani Nikula wrote:
> > >> On Thu, 16 Mar 2017, "Chauhan, Madhav"
> > <madhav.chauhan at intel.com> wrote:
> > >> > > -----Original Message-----
> > >> > > From: Nikula, Jani
> > >> > > Sent: Thursday, February 16, 2017 9:03 PM
> > >> > > To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> > >> > > gfx at lists.freedesktop.org
> > >> > > Cc: Conselvan De Oliveira, Ander
> > >> > > <ander.conselvan.de.oliveira at intel.com>;
> > >> > > Shankar, Uma <uma.shankar at intel.com>; Mukherjee, Indranil
> > >> > > <indranil.mukherjee at intel.com>; Sharma, Shashank
> > >> > > <shashank.sharma at intel.com>; Chauhan, Madhav
> > >> > > <madhav.chauhan at intel.com>; ville.syrjala at linux.intel.com
> > >> > > Subject: Re: [PATCH] drm/i915/glk: CDCLK calculation changes
> > >> > > for glk
> > >> > >
> > >> > > On Thu, 16 Feb 2017, Madhav Chauhan
> > <madhav.chauhan at intel.com>
> > >> > > wrote:
> > >> > > > As per BSPEC, valid cdclk values for glk are 79.2, 158.4, 316.8 Mhz.
> > >> > > > Practically we can achive only 99% of these cdclk values(HW
> > >> > > > team checking on this). So cdclk should be calculated for the
> > >> > > > given pixclk as per that otherwise it may lead to screen
> > >> > > > corruption
> > for some scenarios.
> > >> > > >
> > >> > > > v2: Rebased to new CDLCK code framework
> > >> > > >
> > >> > > > Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> > >> > > > ---
> > >> > > >  drivers/gpu/drm/i915/intel_cdclk.c | 4 ++--
> > >> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >> > > >
> > >> > > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > index d643c0c..834df68 100644
> > >> > > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > >> > > > @@ -1071,9 +1071,9 @@ static int bxt_calc_cdclk(int
> > >> > > > max_pixclk)
> > >> > > >
> > >> > > >  static int glk_calc_cdclk(int max_pixclk)  {
> > >> > > > -	if (max_pixclk > 2 * 158400)
> > >> > > > +	if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
> > >> > >
> > >> > > Where do we ensure we don't use pixel clock 312841..316800?
> > >> > > Clearly we shouldn't use that because we can't guarantee it
> > >> > > works,
> > right?
> > >> >
> > >> > Why do we need to ensure that ?? Can you please elaborate more on
> > this?
> > >> > Here we are finding one of  the defined CDCLK value for a pixel
> > >> > clock
> > >>
> > >> I probably had some great idea a month ago when I wrote that, but I
> > >> can no longer remember what it was. :(
> > >
> > > I'm not sure if that is what you meant, but if the hardware can't
> > > handle it,
> > > intel_compute_max_dotclk() needs to take the 99% limitation into
> > > account
> > too.
> > > I.e., max dot clock would be .99 * 2 *  316800 = 627264.
> >
> > Yes, thank you!

Tested this change on drm-tip. Found that 1-2 times MIPI didn't come up (might be some error 
during testing) after that it works  fine every time.
Could this change have some sort of impact on MIPI functionality?? Looked at code, found nothing
from that front.

> 
> Ok. Will include this change as well along with additional comments for
> explaining 99% usage of cdclk inside glk_calc_cdclk.
> Thanks for review.
> 
> >
> > Jani.
> >
> > >
> > > Ander
> > >
> > >>
> > >> BR,
> > >> Jani.
> > >>
> > >>
> > >> > >
> > >> > > Before we get the spec update to confirm what to do, I think we
> > >> > > need a comment here explaining what's going on.
> > >> >
> > >> > Will add the following comment, if that's fine, will send the
> > >> > rebased
> > patch:
> > >> > "For GLK platform, only 99% of the defined CDCLK value can be
> achieved
> > >> >   So calculate pixel clock on that basis"
> > >> >
> > >> > Regards,
> > >> > Madhav
> > >> > >
> > >> > > BR,
> > >> > > Jani.
> > >> > >
> > >> > > >  		return 316800;
> > >> > > > -	else if (max_pixclk > 2 * 79200)
> > >> > > > +	else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
> > >> > > >  		return 158400;
> > >> > > >  	else
> > >> > > >  		return 79200;
> > >> > >
> > >> > > --
> > >> > > Jani Nikula, Intel Open Source Technology Center
> > >>
> > >>
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center