Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Wed, 05 Apr 2017, Madhav Chauhan 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, explained below:
> 1. For DSI AUO panel(1920x1200 @60) required pixclk is 157100 KHZ
> 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 and causes
>panel corruption.
>
> 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
> v5: Added details of non-working scenario in commit message
>
> Cc: Ander Conselvan de Oliveira
> Cc: Ville Syrjälä
> Signed-off-by: Madhav Chauhan
> Signed-off-by: Jani Nikula
> Reviewed-by: Ander Conselvan de Oliveira
Pushed to drm-intel-next-queued, thanks for the patch and review.
BR,
Jani.
> ---
> 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 dd3ad52..763010f 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
> + * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
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, explained below:
1. For DSI AUO panel(1920x1200 @60) required pixclk is 157100 KHZ
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 and causes
panel corruption.
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
v5: Added details of non-working scenario in commit message
Cc: Ander Conselvan de Oliveira
Cc: Ville Syrjälä
Signed-off-by: Madhav Chauhan
Signed-off-by: Jani Nikula
Reviewed-by: Ander Conselvan de Oliveira
---
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 dd3ad52..763010f 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
+* 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;
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, April 4, 2017 6:26 PM
> To: Chauhan, Madhav
> Cc: Nikula, Jani ; Ander Conselvan De Oliveira
> ; intel-gfx@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 +, Chauhan, Madhav wrote:
> > > -Original Message-
> > > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > > Sent: Tuesday, April 4, 2017 5:09 PM
> > > To: Chauhan, Madhav
> > > Cc: Nikula, Jani ; Ander Conselvan De
> > > Oliveira ; intel-gfx@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 +, Chauhan, Madhav wrote:
> > > > > -Original Message-
> > > > > From: Nikula, Jani
> > > > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > > > To: Ander Conselvan De Oliveira ; intel-
> > > > > g...@lists.freedesktop.org
> > > > > Cc: Chauhan, Madhav ; Ville Syrjälä
> > > > >
> > > > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of
> > > > > cdclk workaround
> > > > >
> > > > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > > >
> > > > > wrote:
> > > > > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > > > >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > > > wrote:
> > > > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > >> > > From: Madhav Chauhan
> > > > > >> > >
> > > > > >> > > 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
> > > > > >> > >
> > > > > >> > > Cc: Ville Syrjälä
> > > > > >> > > Signed-off-by: Madhav Chauhan
>
> > > > > >> > > Signed-off-by: Jani Nikula
> > > > > >> > > ---
> > > > > >> > > 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 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
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, Apr 04, 2017 at 11:53:42AM +, Chauhan, Madhav wrote:
> > -Original Message-
> > From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > Sent: Tuesday, April 4, 2017 5:09 PM
> > To: Chauhan, Madhav
> > Cc: Nikula, Jani ; Ander Conselvan De Oliveira
> > ; intel-gfx@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 +, Chauhan, Madhav wrote:
> > > > -Original Message-
> > > > From: Nikula, Jani
> > > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > > To: Ander Conselvan De Oliveira ; intel-
> > > > g...@lists.freedesktop.org
> > > > Cc: Chauhan, Madhav ; Ville Syrjälä
> > > >
> > > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > > > workaround
> > > >
> > > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > >
> > > > wrote:
> > > > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > > >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > > wrote:
> > > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > >> > > From: Madhav Chauhan
> > > > >> > >
> > > > >> > > 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
> > > > >> > >
> > > > >> > > Cc: Ville Syrjälä
> > > > >> > > Signed-off-by: Madhav Chauhan
> > > > >> > > Signed-off-by: Jani Nikula
> > > > >> > > ---
> > > > >> > > 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 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?
> 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
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
> -Original Message-
> From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> Sent: Tuesday, April 4, 2017 5:09 PM
> To: Chauhan, Madhav
> Cc: Nikula, Jani ; Ander Conselvan De Oliveira
> ; intel-gfx@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 +, Chauhan, Madhav wrote:
> > > -Original Message-
> > > From: Nikula, Jani
> > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > To: Ander Conselvan De Oliveira ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: Chauhan, Madhav ; Ville Syrjälä
> > >
> > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > > workaround
> > >
> > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > >
> > > wrote:
> > > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > > wrote:
> > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > >> > > From: Madhav Chauhan
> > > >> > >
> > > >> > > 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
> > > >> > >
> > > >> > > Cc: Ville Syrjälä
> > > >> > > Signed-off-by: Madhav Chauhan
> > > >> > > Signed-off-by: Jani Nikula
> > > >> > > ---
> > > >> > > 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 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)
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.
>
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
> -Original Message-
> From: Ander Conselvan De Oliveira [mailto:conselv...@gmail.com]
> Sent: Tuesday, April 4, 2017 4:13 PM
> To: Chauhan, Madhav ; Nikula, Jani
> ; intel-gfx@lists.freedesktop.org
> Cc: Ville Syrjälä
> Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> workaround
>
> On Tue, 2017-04-04 at 10:27 +, Chauhan, Madhav wrote:
> > > -Original Message-
> > > From: Nikula, Jani
> > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > To: Ander Conselvan De Oliveira ; intel-
> > > g...@lists.freedesktop.org
> > > Cc: Chauhan, Madhav ; Ville Syrjälä
> > >
> > > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > > workaround
> > >
> > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > >
> > > wrote:
> > > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > >
> > > wrote:
> > > > > > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > > > From: Madhav Chauhan
> > > > > > >
> > > > > > > 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
> > > > > > >
> > > > > > > Cc: Ville Syrjälä
> > > > > > > Signed-off-by: Madhav Chauhan
> > > > > > > Signed-off-by: Jani Nikula
> > > > > > > ---
> > > > > > > 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 panel (19X12) required pixclk is 157100 KHZ 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??
>
> If someone else needs to deal with this six months or a year from now and
> you, me and Jani are nowhere to be found, the code or commit message
> should still have enough information to let that person assess whether it is
> safe to remove the workaround or not. Having the model number just makes
> it easier to determine what real world scenario triggers this problem, so it
> is
> easier to test a fix.
Agree from this perspective. Will add this explanation in commit message.
>
> Ander
>
> >
> > >
> > >
> > > >
> > > > Ander
> > > >
> > > >
> > > > > > With that fixed,
> > > > > >
> > > > > > Reviewed-by: Ander Conselvan de Oliveira
> > > > > >
> > > > > >
> > > > > > > + * intel_compute_max_dotclk() limits the max pixel clock
> > > > > > > +to 99% of
> > >
> > > max
> > > > > > > + * cdclk.)
> > > > > > >
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, Apr 04, 2017 at 10:27:57AM +, Chauhan, Madhav wrote:
> > -Original Message-
> > From: Nikula, Jani
> > Sent: Tuesday, April 4, 2017 3:48 PM
> > To: Ander Conselvan De Oliveira ; intel-
> > g...@lists.freedesktop.org
> > Cc: Chauhan, Madhav ; Ville Syrjälä
> >
> > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > workaround
> >
> > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > wrote:
> > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > wrote:
> > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > >> > > From: Madhav Chauhan
> > >> > >
> > >> > > 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
> > >> > >
> > >> > > Cc: Ville Syrjälä
> > >> > > Signed-off-by: Madhav Chauhan
> > >> > > Signed-off-by: Jani Nikula
> > >> > > ---
> > >> > > 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 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.
And if the problem is really that we can't reach 100% of cdclk, then
why are we limiting this to GLK only?
> 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
> > >> >
> > >> > > + * 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
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, 2017-04-04 at 10:27 +, Chauhan, Madhav wrote:
> > -Original Message-
> > From: Nikula, Jani
> > Sent: Tuesday, April 4, 2017 3:48 PM
> > To: Ander Conselvan De Oliveira ; intel-
> > g...@lists.freedesktop.org
> > Cc: Chauhan, Madhav ; Ville Syrjälä
> >
> > Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> > workaround
> >
> > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> > wrote:
> > > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> > > > On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> >
> > wrote:
> > > > > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > > From: Madhav Chauhan
> > > > > >
> > > > > > 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
> > > > > >
> > > > > > Cc: Ville Syrjälä
> > > > > > Signed-off-by: Madhav Chauhan
> > > > > > Signed-off-by: Jani Nikula
> > > > > > ---
> > > > > > 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 panel (19X12) required pixclk is 157100 KHZ
> 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??
If someone else needs to deal with this six months or a year from now and you,
me and Jani are nowhere to be found, the code or commit message should still
have enough information to let that person assess whether it is safe to remove
the workaround or not. Having the model number just makes it easier to determine
what real world scenario triggers this problem, so it is easier to test a fix.
Ander
>
> >
> >
> > >
> > > Ander
> > >
> > >
> > > > > With that fixed,
> > > > >
> > > > > Reviewed-by: Ander Conselvan de Oliveira
> > > > >
> > > > > > +* 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
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
> -Original Message-
> From: Nikula, Jani
> Sent: Tuesday, April 4, 2017 3:48 PM
> To: Ander Conselvan De Oliveira ; intel-
> g...@lists.freedesktop.org
> Cc: Chauhan, Madhav ; Ville Syrjälä
>
> Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> workaround
>
> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> wrote:
> > On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> >> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
> wrote:
> >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> >> > > From: Madhav Chauhan
> >> > >
> >> > > 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
> >> > >
> >> > > Cc: Ville Syrjälä
> >> > > Signed-off-by: Madhav Chauhan
> >> > > Signed-off-by: Jani Nikula
> >> > > ---
> >> > > 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 panel (19X12) required pixclk is 157100 KHZ
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
> >> >
> >> > > + * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, 04 Apr 2017, Ander Conselvan De Oliveira wrote:
> On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
>> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira
>> wrote:
>> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
>> > > From: Madhav Chauhan
>> > >
>> > > 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
>> > > Cc: Ville Syrjälä
>> > > Signed-off-by: Madhav Chauhan
>> > > Signed-off-by: Jani Nikula
>> > > ---
>> > > 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?
>
> Ander
>
>
>> > With that fixed,
>> >
>> > Reviewed-by: Ander Conselvan de Oliveira
>> >
>> > > + * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira wrote:
> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > From: Madhav Chauhan
> > >
> > > 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
> > > Cc: Ville Syrjälä
> > > Signed-off-by: Madhav Chauhan
> > > Signed-off-by: Jani Nikula
> > > ---
> > > 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.
Ander
> > With that fixed,
> >
> > Reviewed-by: Ander Conselvan de Oliveira
> >
> > > + * 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;
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, 04 Apr 2017, Ander Conselvan De Oliveira wrote:
> On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
>> From: Madhav Chauhan
>>
>> 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
>> Cc: Ville Syrjälä
>> Signed-off-by: Madhav Chauhan
>> Signed-off-by: Jani Nikula
>> ---
>> 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.
BR,
Jani.
>
> With that fixed,
>
> Reviewed-by: Ander Conselvan de Oliveira
>
>> + * 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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> From: Madhav Chauhan
>
> 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
> Cc: Ville Syrjälä
> Signed-off-by: Madhav Chauhan
> Signed-off-by: Jani Nikula
> ---
> 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.
With that fixed,
Reviewed-by: Ander Conselvan de Oliveira
> + * 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;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk workaround
From: Madhav Chauhan
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
Cc: Ville Syrjälä
Signed-off-by: Madhav Chauhan
Signed-off-by: Jani Nikula
---
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
+* 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;
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
