[Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround

Tue Apr 4 13:26:51 UTC 2017

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Tuesday, April 4, 2017 6:26 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>
> Cc: Nikula, Jani <jani.nikula at intel.com>; Ander Conselvan De Oliveira
> <conselvan2 at gmail.com>; intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> workaround
> 
> On Tue, Apr 04, 2017 at 11:53:42AM +0000, Chauhan, Madhav wrote:
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> > > Sent: Tuesday, April 4, 2017 5:09 PM
> > > To: Chauhan, Madhav <madhav.chauhan at intel.com>
> > > Cc: Nikula, Jani <jani.nikula at intel.com>; Ander Conselvan De
> > > Oliveira <conselvan2 at gmail.com>; intel-gfx at lists.freedesktop.org
> > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > > workaround
> > >
> > > On Tue, Apr 04, 2017 at 10:27:57AM +0000, Chauhan, Madhav wrote:
> > > > > -----Original Message-----
> > > > > From: Nikula, Jani
> > > > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > > > To: Ander Conselvan De Oliveira <conselvan2 at gmail.com>; intel-
> > > > > gfx at lists.freedesktop.org
> > > > > Cc: Chauhan, Madhav <madhav.chauhan at intel.com>; Ville Syrjälä
> > > > > <ville.syrjala at linux.intel.com>
> > > > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of
> > > > > cdclk workaround
> > > > >
> > > > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > > > <conselvan2 at gmail.com>
> > > > > wrote:
> > > > > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > > > >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > > > <conselvan2 at gmail.com> wrote:
> > > > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > >> > > From: Madhav Chauhan <madhav.chauhan at intel.com>
> > > > > >> > >
> > > > > >> > > 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
> > > > > >> > > v3: Addressed review comments from Ander/Jani
> > > > > >> > >     - Add comment in code about 99% usage of CDCLK
> > > > > >> > >     - Calculate max dot clock as well with 99% limit
> > > > > >> > > v4 by Jani:
> > > > > >> > >     - drop superfluous whitespace change
> > > > > >> > >     - rewrite code comments to clarify
> > > > > >> > >
> > > > > >> > > Cc: Ander Conselvan de Oliveira
> > > > > >> > > <ander.conselvan.de.oliveira at intel.com>
> > > > > >> > > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > >> > > Signed-off-by: Madhav Chauhan
> <madhav.chauhan at intel.com>
> > > > > >> > > Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> > > > > >> > > ---
> > > > > >> > >  drivers/gpu/drm/i915/intel_cdclk.c | 16 +++++++++++++---
> > > > > >> > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > > > > >> > >
> > > > > >> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > >> > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > >> > > index dd3ad52b7dfe..763010f8ad89 100644
> > > > > >> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > >> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > > > >> > > @@ -1071,9 +1071,15 @@ static int bxt_calc_cdclk(int
> > > > > >> > > max_pixclk)
> > > > > >> > >
> > > > > >> > >  static int glk_calc_cdclk(int max_pixclk)  {
> > > > > >> > > -	if (max_pixclk > 2 * 158400)
> > > > > >> > > +	/*
> > > > > >> > > +	 * FIXME: Avoid using a pixel clock that is more than
> > > > > >> > > +99% of
> > > the cdclk
> > > > > >> > > +	 * as a temporary workaround. Use a higher cdclk instead.
> > > > > >> > > +(Note that
> > > > > >> >
> > > > > >> > Temporary workaround for what? Neither the comment nor the
> > > > > >> > commit message explicitly lists the scenario that triggers this
> issue.
> > > > > >>
> > > > > >> Frankly I did not know what to write.
> > > > > >
> > > > > >
> > > > > >> There are issues with pixel clocks near cdclk that shouldn't
> > > > > >> happen. People are looking into this, but in the mean time
> > > > > >> dodge the issues by using higher cdclk. The issue should not
> > > > > >> be encoder specific, but in practice this has only been seen
> > > > > >> with DSI because there were some modes with pixel clocks that
> > > > > >> are near the cdclk.
> > > > > >
> > > > > > The above plus the model number of the DSI panel with which
> > > > > > the issue has been seen would be good enough IMO.
> > > > >
> > > > > Madhav?
> > > >
> > > > Issue is generic (valid for DP/HDMI) not just MIPI DSI specific.
> > > > Particular DSI panel exposing this issue as described below:
> > > > 1. For XXXX panel (19X12) required pixclk is 157100 KHZ
> > >
> > > And what is the actual pixel clock we end up using? As I've
> > > mentioned before our code is buggy because it assumes we can get any
> > > arbitrary clock frequency from the PLL. That is not generally true.
> > >
> > > So I'd like to know if the problem really is that we can't reach
> > > 100% of cdclk, or if we just end up with a pixel clock that slightly
> > > higher than what we asked for and thus exceeds 100% of cdclk.
> >
> > AFAIK (from testing, GOP team inputs), problem is that we can't reach
> 100%CDCLK on GLK.
> > In this scenario calculated/required pixel clock is 157100 KHZ  (might
> > be higher due to buggy code as you mentioned)
> 
> Well, what is the clock we get from the PLL exactly?
Please explain more if I misunderstood your question.
For DSI, we get pclk for a panel by VBT parsing. On the basis of this pclk we 
Program PLL dividers  and then enable the PLL to generate the required clk.
Do you mean this pclk??

> 
> > which should be handled very well if we set CDCLK 79200 KHZ but causes
> panel corruption.
> >
> > >
> > > And if the problem is really that we can't reach 100% of cdclk, then
> > > why are we limiting this to GLK only?
> >
> > From discussion, looks like issue is specific to GLK platform.
> > HW team is debugging this issue.
> >
> > >
> > > > 2. glk_calc_cdclk returns 79200 KHZ for this pixclk. For 2PPC it
> > > > will be 158400 KHZ 3. Practically 100% of the cdclk can’t be
> > > > achieved, so 99% of
> > > 158400 KHZ  = 156816 which is less than the desired pixlclk.
> > > >      This causes the corruption.
> > > >
> > > > So why do we need to mention a particular panel??
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > > Ander
> > > > > >
> > > > > >
> > > > > >> > With that fixed,
> > > > > >> >
> > > > > >> > Reviewed-by: Ander Conselvan de Oliveira
> > > > > >> > <conselvan2 at gmail.com>
> > > > > >> >
> > > > > >> > > +	 * intel_compute_max_dotclk() limits the max pixel
> > > > > >> > > +clock to 99% of
> > > > > max
> > > > > >> > > +	 * cdclk.)
> > > > > >> > > +	 */
> > > > > >> > > +	if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
> > > > > >> > >  		return 316800;
> > > > > >> > > -	else if (max_pixclk > 2 * 79200)
> > > > > >> > > +	else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99,
> > > > > >> > > +100))
> > > > > >> > >  		return 158400;
> > > > > >> > >  	else
> > > > > >> > >  		return 79200;
> > > > > >> > > @@ -1664,7 +1670,11 @@ static int
> > > > > >> > > intel_compute_max_dotclk(struct
> > > > > drm_i915_private *dev_priv)
> > > > > >> > >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > > > > >> > >
> > > > > >> > >  	if (IS_GEMINILAKE(dev_priv))
> > > > > >> > > -		return 2 * max_cdclk_freq;
> > > > > >> > > +		/*
> > > > > >> > > +		 * FIXME: Limiting to 99% as a temporary
> > > workaround. See
> > > > > >> > > +		 * glk_calc_cdclk() for details.
> > > > > >> > > +		 */
> > > > > >> > > +		return 2 * max_cdclk_freq * 99 / 100;
> > > > > >> > >  	else if (INTEL_INFO(dev_priv)->gen >= 9 ||
> > > > > >> > >  		 IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > > > > >> > >  		return max_cdclk_freq;
> > > > > >>
> > > > > >>
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel Open Source Technology Center
> > >
> > > --
> > > Ville Syrjälä
> > > Intel OTC
> 
> --
> Ville Syrjälä
> Intel OTC