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

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

[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, 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 <ander.conselvan.de.oliveira@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.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 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

✓ Fi.CI.BAT: success for drm/i915/glk: limit pixel clock to 99% of cdclk workaround (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/glk: limit pixel clock to 99% of cdclk workaround (rev2)
URL   : https://patchwork.freedesktop.org/series/22404/
State : success

== Summary ==

Series 22404v2 drm/i915/glk: limit pixel clock to 99% of cdclk workaround
https://patchwork.freedesktop.org/api/1.0/series/22404/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                incomplete -> PASS       (fi-snb-2520m)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 429s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 425s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time: 581s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 503s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 547s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time: 485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 481s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 410s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 400s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 419s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 465s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 457s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time: 564s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 456s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 568s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 459s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 483s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 539s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 404s

3c5f424b9e2697352118ee33514b29b7cd57f69c drm-tip: 2017y-04m-05d-11h-46m-45s UTC integration manifest
6dbd9dc drm/i915/glk: limit pixel clock to 99% of cdclk workaround

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4408/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Wed, 05 Apr 2017, Madhav Chauhan <madhav.chauhan@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, 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 <ander.conselvan.de.oliveira@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

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

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

[Jani Nikula]

From: Madhav Chauhan <madhav.chauhan@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@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@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
+	 * 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

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

[Ander Conselvan De Oliveira]

On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@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.

With that fixed,

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Tue, 04 Apr 2017, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
>> From: Madhav Chauhan <madhav.chauhan@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@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@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.

BR,
Jani.


>
> With that fixed,
>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ander Conselvan De Oliveira]

On Tue, 2017-04-04 at 11:40 +0300, Jani Nikula wrote:
> On Tue, 04 Apr 2017, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > Signed-off-by: Jani Nikula <jani.nikula@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.

Ander


> > With that fixed,
> > 
> > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@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;
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Tue, 04 Apr 2017, Ander Conselvan De Oliveira <conselvan2@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@gmail.com> wrote:
>> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
>> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
>> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
>> > > Signed-off-by: Jani Nikula <jani.nikula@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?


>
> Ander
>
>
>> > With that fixed,
>> > 
>> > Reviewed-by: Ander Conselvan de Oliveira <conselvan2@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, April 4, 2017 3:48 PM
> To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> <ville.syrjala@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@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@gmail.com> wrote:
> >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> >> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> >> > > Signed-off-by: Jani Nikula <jani.nikula@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
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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ander Conselvan De Oliveira]

On Tue, 2017-04-04 at 10:27 +0000, Chauhan, Madhav wrote:
> > -----Original Message-----
> > From: Nikula, Jani
> > Sent: Tuesday, April 4, 2017 3:48 PM
> > To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > <ville.syrjala@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@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@gmail.com> wrote:
> > > > > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > > > > Signed-off-by: Jani Nikula <jani.nikula@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
> 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 <conselvan2@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chauhan, Madhav]

> -----Original Message-----
> From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
> Sent: Tuesday, April 4, 2017 4:13 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Subject: Re: [PATCH] drm/i915/glk: limit pixel clock to 99% of cdclk
> workaround
> 
> On Tue, 2017-04-04 at 10:27 +0000, Chauhan, Madhav wrote:
> > > -----Original Message-----
> > > From: Nikula, Jani
> > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > > <ville.syrjala@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@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@gmail.com> wrote:
> > > > > > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > > > > > Signed-off-by: Jani Nikula <jani.nikula@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 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
> > > > > > <conselvan2@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

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@gmail.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > <ville.syrjala@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@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@gmail.com> wrote:
> > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > >> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > >> > > Signed-off-by: Jani Nikula <jani.nikula@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.

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 <conselvan2@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chauhan, Madhav]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Tuesday, April 4, 2017 5:09 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>
> Cc: Nikula, Jani <jani.nikula@intel.com>; Ander Conselvan De Oliveira
> <conselvan2@gmail.com>; 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 +0000, Chauhan, Madhav wrote:
> > > -----Original Message-----
> > > From: Nikula, Jani
> > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > > <ville.syrjala@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@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@gmail.com> wrote:
> > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > >> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > >> > > Signed-off-by: Jani Nikula <jani.nikula@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)
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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Ville Syrjälä]

On Tue, Apr 04, 2017 at 11:53:42AM +0000, Chauhan, Madhav wrote:
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Tuesday, April 4, 2017 5:09 PM
> > To: Chauhan, Madhav <madhav.chauhan@intel.com>
> > Cc: Nikula, Jani <jani.nikula@intel.com>; Ander Conselvan De Oliveira
> > <conselvan2@gmail.com>; 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 +0000, Chauhan, Madhav wrote:
> > > > -----Original Message-----
> > > > From: Nikula, Jani
> > > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > > To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> > > > gfx@lists.freedesktop.org
> > > > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > > > <ville.syrjala@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@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@gmail.com> wrote:
> > > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > >> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > > >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > >> > > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > > > >> > > Signed-off-by: Jani Nikula <jani.nikula@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?

> 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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chauhan, Madhav]

> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Tuesday, April 4, 2017 6:26 PM
> To: Chauhan, Madhav <madhav.chauhan@intel.com>
> Cc: Nikula, Jani <jani.nikula@intel.com>; Ander Conselvan De Oliveira
> <conselvan2@gmail.com>; 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 +0000, Chauhan, Madhav wrote:
> > > -----Original Message-----
> > > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > > Sent: Tuesday, April 4, 2017 5:09 PM
> > > To: Chauhan, Madhav <madhav.chauhan@intel.com>
> > > Cc: Nikula, Jani <jani.nikula@intel.com>; Ander Conselvan De
> > > Oliveira <conselvan2@gmail.com>; 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 +0000, Chauhan, Madhav wrote:
> > > > > -----Original Message-----
> > > > > From: Nikula, Jani
> > > > > Sent: Tuesday, April 4, 2017 3:48 PM
> > > > > To: Ander Conselvan De Oliveira <conselvan2@gmail.com>; intel-
> > > > > gfx@lists.freedesktop.org
> > > > > Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Ville Syrjälä
> > > > > <ville.syrjala@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@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@gmail.com> wrote:
> > > > > >> > On Tue, 2017-04-04 at 11:15 +0300, Jani Nikula wrote:
> > > > > >> > > From: Madhav Chauhan <madhav.chauhan@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@intel.com>
> > > > > >> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > >> > > Signed-off-by: Madhav Chauhan
> <madhav.chauhan@intel.com>
> > > > > >> > > Signed-off-by: Jani Nikula <jani.nikula@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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx