[PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Hans de Goede]

At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image
shifted aprox. 20% to the left (with wraparound) and sometimes also wrong
colors, showing that the panel controller is starting with sampling the
datastream somewhere mid-line. This happens after the first blanking and
re-init of the panel.

After looking at drm.debug output I noticed that initially we inherit the
cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667
KHz, which turns out to be the cause of this problem, a quick hack to hard
code the cdclk to 333333 KHz makes the problem go away.

I've tested this on various Bay Trail devices:
-Chuwi Vi8: 800x1280, 333333 at boot, requires 333333 to work properly
-GP-electronic T701: 1024x600, 333333 at boot, works fine with 333333
-PEAQ C1010: 1920x1200, 333333 at boot, works fine with 333333
-PoV mobii-wintab-800w: 800x1280, 333333 at boot, works fine with 333333
-Asus Transformer-T100TA: 1368x768, 320000 at boot, works fine with 320000

So it seems that the GOP is always using what vlv_calc_cdclk calls
freq_320 as cdclk clock. Also note the comment in vlv_calc_cdclk about
it avoiding 200 Mhz as clock because that causes issues in some cases.

This commit extends the "do not use 200 Mhz" workaround with an extra
check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI
panel is active.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9c5ceb98d48f..cfb3f7fb3e1c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1923,6 +1923,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
+	/*
+	 * On Valleyview the GOP always uses 320000/333333 KHz if DSI is used,
+	 * using a lower clock causes causes sync issues with some panels.
+	 */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
+	    IS_VALLEYVIEW(dev_priv))
+		min_cdclk = max(320000, min_cdclk);
+
 	if (min_cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
 			      min_cdclk, dev_priv->max_cdclk_freq);
-- 
2.14.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Hans de Goede]

Hi,

I forgot to add a coverletter, anyways what I wanted to put in the
coverletter and not in the commit message is a link to a picture of the
problem this fixes:

https://fedorapeople.org/~jwrdegoede/IMG_20171217_195637.jpg

Note the screen is actually a portrait screen, so the picture is the
right way up. Beside the obvious left shift with wrap-around of the
picture, also all the green in there is supposed to be blue.

Another less clear picture is this one:

https://fedorapeople.org/~jwrdegoede/IMG_20171217_195507.jpg

Again with wrong colors.

###

While working on this I also noticed that the pixelclock as
the driver gets it from the VBT is not the same as the one the
GOP uses, on the tablet in question the VBT says 77000 KHz
and the GOP uses (according to the initial readback) 78125 KHz,
on another tablet I noticed the VBT saying 78125 KHz, where
as the GOP was using 68xxx KHz which came to a refresh-rate
of around 60 Hz, where as the VBT value is 69 Hz IIRC.

Neither of these cause any actual issue (fixing the pixelclock
to match the GOP programmed value does not fix the issue, where
as using the GOP cdclk of 333333KHz does).

Regards,

Hans



On 19-12-17 20:21, Hans de Goede wrote:
> At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image
> shifted aprox. 20% to the left (with wraparound) and sometimes also wrong
> colors, showing that the panel controller is starting with sampling the
> datastream somewhere mid-line. This happens after the first blanking and
> re-init of the panel.
> 
> After looking at drm.debug output I noticed that initially we inherit the
> cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667
> KHz, which turns out to be the cause of this problem, a quick hack to hard
> code the cdclk to 333333 KHz makes the problem go away.
> 
> I've tested this on various Bay Trail devices:
> -Chuwi Vi8: 800x1280, 333333 at boot, requires 333333 to work properly
> -GP-electronic T701: 1024x600, 333333 at boot, works fine with 333333
> -PEAQ C1010: 1920x1200, 333333 at boot, works fine with 333333
> -PoV mobii-wintab-800w: 800x1280, 333333 at boot, works fine with 333333
> -Asus Transformer-T100TA: 1368x768, 320000 at boot, works fine with 320000
> 
> So it seems that the GOP is always using what vlv_calc_cdclk calls
> freq_320 as cdclk clock. Also note the comment in vlv_calc_cdclk about
> it avoiding 200 Mhz as clock because that causes issues in some cases.
> 
> This commit extends the "do not use 200 Mhz" workaround with an extra
> check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI
> panel is active.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 9c5ceb98d48f..cfb3f7fb3e1c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1923,6 +1923,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>   	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>   		min_cdclk = max(2 * 96000, min_cdclk);
>   
> +	/*
> +	 * On Valleyview the GOP always uses 320000/333333 KHz if DSI is used,
> +	 * using a lower clock causes causes sync issues with some panels.
> +	 */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> +	    IS_VALLEYVIEW(dev_priv))
> +		min_cdclk = max(320000, min_cdclk);
> +
>   	if (min_cdclk > dev_priv->max_cdclk_freq) {
>   		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
>   			      min_cdclk, dev_priv->max_cdclk_freq);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Rodrigo Vivi]

On Tue, Dec 19, 2017 at 07:21:13PM +0000, Hans de Goede wrote:
> At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image
> shifted aprox. 20% to the left (with wraparound) and sometimes also wrong
> colors, showing that the panel controller is starting with sampling the
> datastream somewhere mid-line. This happens after the first blanking and
> re-init of the panel.
> 
> After looking at drm.debug output I noticed that initially we inherit the
> cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667
> KHz, which turns out to be the cause of this problem, a quick hack to hard
> code the cdclk to 333333 KHz makes the problem go away.
> 
> I've tested this on various Bay Trail devices:
> -Chuwi Vi8: 800x1280, 333333 at boot, requires 333333 to work properly
> -GP-electronic T701: 1024x600, 333333 at boot, works fine with 333333
> -PEAQ C1010: 1920x1200, 333333 at boot, works fine with 333333
> -PoV mobii-wintab-800w: 800x1280, 333333 at boot, works fine with 333333
> -Asus Transformer-T100TA: 1368x768, 320000 at boot, works fine with 320000

and lower clock shift the image in all of them?

> 
> So it seems that the GOP is always using what vlv_calc_cdclk calls
> freq_320 as cdclk clock. Also note the comment in vlv_calc_cdclk about
> it avoiding 200 Mhz as clock because that causes issues in some cases.
> 
> This commit extends the "do not use 200 Mhz" workaround with an extra
> check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI
> panel is active.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 9c5ceb98d48f..cfb3f7fb3e1c 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1923,6 +1923,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>  	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>  		min_cdclk = max(2 * 96000, min_cdclk);
>  
> +	/*
> +	 * On Valleyview the GOP always uses 320000/333333 KHz if DSI is used,
> +	 * using a lower clock causes causes sync issues with some panels.
> +	 */

Afaik (afai-have-noticed) GOP always use the higher freq and higher rates
that panels support with no care about saving any power. So I'm not sure if
that should be used as any sort of reference for us.

Note I'm not telling to not add the workaround itself...

> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> +	    IS_VALLEYVIEW(dev_priv))
> +		min_cdclk = max(320000, min_cdclk);
> +
>  	if (min_cdclk > dev_priv->max_cdclk_freq) {
>  		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
>  			      min_cdclk, dev_priv->max_cdclk_freq);
> -- 
> 2.14.3
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.BAT: success for drm/i915/vlv: Add cdclk workaround for DSI

[Patchwork]

== Series Details ==

Series: drm/i915/vlv: Add cdclk workaround for DSI
URL   : https://patchwork.freedesktop.org/series/35598/
State : success

== Summary ==

Series 35598v1 drm/i915/vlv: Add cdclk workaround for DSI
https://patchwork.freedesktop.org/api/1.0/series/35598/revisions/1/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#104172 +1
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                dmesg-warn -> PASS       (fi-skl-6700hq) fdo#101144

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#101144 https://bugs.freedesktop.org/show_bug.cgi?id=101144

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:492s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:489s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:502s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:477s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:264s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:412s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:383s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:484s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:427s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:477s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:518s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:469s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:527s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:575s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:442s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:533s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:554s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:558s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:409s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:593s
fi-cnl-y         total:288  pass:261  dwarn:1   dfail:0   fail:0   skip:26  time:620s
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_7541/fi-bdw-gvtdvm/igt.log

cd7e14499b1dbbcb6ea97695ebe39e60b5200cc8 drm-tip: 2017y-12m-19d-15h-09m-52s UTC integration manifest
72e663cabae3 drm/i915/vlv: Add cdclk workaround for DSI

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7541/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Ville Syrjälä]

On Tue, Dec 19, 2017 at 08:31:07PM +0100, Hans de Goede wrote:
> Hi,
> 
> I forgot to add a coverletter, anyways what I wanted to put in the
> coverletter and not in the commit message is a link to a picture of the
> problem this fixes:
> 
> https://fedorapeople.org/~jwrdegoede/IMG_20171217_195637.jpg
> 
> Note the screen is actually a portrait screen, so the picture is the
> right way up. Beside the obvious left shift with wrap-around of the
> picture, also all the green in there is supposed to be blue.
> 
> Another less clear picture is this one:
> 
> https://fedorapeople.org/~jwrdegoede/IMG_20171217_195507.jpg
> 
> Again with wrong colors.
> 
> ###
> 
> While working on this I also noticed that the pixelclock as
> the driver gets it from the VBT is not the same as the one the
> GOP uses, on the tablet in question the VBT says 77000 KHz
> and the GOP uses (according to the initial readback) 78125 KHz,
> on another tablet I noticed the VBT saying 78125 KHz, where
> as the GOP was using 68xxx KHz which came to a refresh-rate
> of around 60 Hz, where as the VBT value is 69 Hz IIRC.
> 
> Neither of these cause any actual issue (fixing the pixelclock
> to match the GOP programmed value does not fix the issue, where
> as using the GOP cdclk of 333333KHz does).

https://patchwork.freedesktop.org/patch/127189/

I suppose it's unlike that would help here since it's just about the 
overlap in dual link mode, but there could be other fail in the DSI
clock code.


-- 
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/vlv: Add cdclk workaround for DSI

[Hans de Goede]

Hi,

On 19-12-17 20:42, Rodrigo Vivi wrote:
> 
> On Tue, Dec 19, 2017 at 07:21:13PM +0000, Hans de Goede wrote:
>> At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image
>> shifted aprox. 20% to the left (with wraparound) and sometimes also wrong
>> colors, showing that the panel controller is starting with sampling the
>> datastream somewhere mid-line. This happens after the first blanking and
>> re-init of the panel.
>>
>> After looking at drm.debug output I noticed that initially we inherit the
>> cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667
>> KHz, which turns out to be the cause of this problem, a quick hack to hard
>> code the cdclk to 333333 KHz makes the problem go away.
>>
>> I've tested this on various Bay Trail devices:
>> -Chuwi Vi8: 800x1280, 333333 at boot, requires 333333 to work properly
>> -GP-electronic T701: 1024x600, 333333 at boot, works fine with 333333
>> -PEAQ C1010: 1920x1200, 333333 at boot, works fine with 333333
>> -PoV mobii-wintab-800w: 800x1280, 333333 at boot, works fine with 333333
>> -Asus Transformer-T100TA: 1368x768, 320000 at boot, works fine with 320000
> 
> and lower clock shift the image in all of them?

No, just on the Chuwi Vi8, the tests was to make sure the higher clock
does not create problems elsewhere and I was curious what clock the GOP
was using on other devices. Also hence the "requires 333333 to work properly"
vs "works fine with 333333" in the list.

>> So it seems that the GOP is always using what vlv_calc_cdclk calls
>> freq_320 as cdclk clock. Also note the comment in vlv_calc_cdclk about
>> it avoiding 200 Mhz as clock because that causes issues in some cases.
>>
>> This commit extends the "do not use 200 Mhz" workaround with an extra
>> check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI
>> panel is active.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
>> index 9c5ceb98d48f..cfb3f7fb3e1c 100644
>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>> @@ -1923,6 +1923,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>>   	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>>   		min_cdclk = max(2 * 96000, min_cdclk);
>>   
>> +	/*
>> +	 * On Valleyview the GOP always uses 320000/333333 KHz if DSI is used,
>> +	 * using a lower clock causes causes sync issues with some panels.
>> +	 */
> 
> Afaik (afai-have-noticed) GOP always use the higher freq and higher rates
> that panels support with no care about saving any power. So I'm not sure if
> that should be used as any sort of reference for us.
> 
> Note I'm not telling to not add the workaround itself...

So you're suggesting to change the comment into something like this
instead ?  :

/*
  * On Valleyview some DSI panels loose (v|h)sync when the clock is lower
  * then 320000KHz.
  */

> 
>> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
>> +	    IS_VALLEYVIEW(dev_priv))
>> +		min_cdclk = max(320000, min_cdclk);
>> +
>>   	if (min_cdclk > dev_priv->max_cdclk_freq) {
>>   		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
>>   			      min_cdclk, dev_priv->max_cdclk_freq);
>> -- 
>> 2.14.3
>>

Regards,

Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Hans de Goede]

Hi,

On 19-12-17 20:48, Ville Syrjälä wrote:
> On Tue, Dec 19, 2017 at 08:31:07PM +0100, Hans de Goede wrote:
>> Hi,
>>
>> I forgot to add a coverletter, anyways what I wanted to put in the
>> coverletter and not in the commit message is a link to a picture of the
>> problem this fixes:
>>
>> https://fedorapeople.org/~jwrdegoede/IMG_20171217_195637.jpg
>>
>> Note the screen is actually a portrait screen, so the picture is the
>> right way up. Beside the obvious left shift with wrap-around of the
>> picture, also all the green in there is supposed to be blue.
>>
>> Another less clear picture is this one:
>>
>> https://fedorapeople.org/~jwrdegoede/IMG_20171217_195507.jpg
>>
>> Again with wrong colors.
>>
>> ###
>>
>> While working on this I also noticed that the pixelclock as
>> the driver gets it from the VBT is not the same as the one the
>> GOP uses, on the tablet in question the VBT says 77000 KHz
>> and the GOP uses (according to the initial readback) 78125 KHz,
>> on another tablet I noticed the VBT saying 78125 KHz, where
>> as the GOP was using 68xxx KHz which came to a refresh-rate
>> of around 60 Hz, where as the VBT value is 69 Hz IIRC.
>>
>> Neither of these cause any actual issue (fixing the pixelclock
>> to match the GOP programmed value does not fix the issue, where
>> as using the GOP cdclk of 333333KHz does).
> 
> https://patchwork.freedesktop.org/patch/127189/
> 
> I suppose it's unlike that would help here since it's just about the
> overlap in dual link mode, but there could be other fail in the DSI
> clock code.

Yes that is is not applicable to the Vi8 where I'm seeing the
77000 KHz used by GOP vs 78125 KHz used by i915.

Looking at intel_dsi_vbt_init the pclk on the boards where I'm
seeing an inconsistency is just taken directly from mode->clock,
which in turn is simply dvo_timing->clock * 10.

Again: these different pixel-clocks do not seem to be an issue,
I just noticed this too while looking at the problem best seen
here:

https://fedorapeople.org/~jwrdegoede/IMG_20171217_195637.jpg

Regards,

Hans
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915/vlv: Add cdclk workaround for DSI

[Patchwork]

== Series Details ==

Series: drm/i915/vlv: Add cdclk workaround for DSI
URL   : https://patchwork.freedesktop.org/series/35598/
State : success

== Summary ==

Test kms_flip:
        Subgroup vblank-vs-modeset-suspend:
                skip       -> PASS       (shard-snb) fdo#102365
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623
Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_draw_crc:
        Subgroup draw-method-xrgb2101010-mmap-gtt-untiled:
                skip       -> PASS       (shard-snb)
Test kms_busy:
        Subgroup extended-pageflip-hang-oldfb-render-b:
                skip       -> PASS       (shard-snb)
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-nonblocking:
                skip       -> PASS       (shard-snb)
Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-snb) fdo#104058

fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058

shard-hsw        total:2712 pass:1537 dwarn:1   dfail:0   fail:10  skip:1164 time:9415s
shard-snb        total:2712 pass:1308 dwarn:2   dfail:0   fail:11  skip:1391 time:8123s
Blacklisted hosts:
shard-kbl        total:2640 pass:1766 dwarn:1   dfail:0   fail:25  skip:847 time:10944s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7541/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/vlv: Add cdclk workaround for DSI

[Rodrigo Vivi]

On Tue, Dec 19, 2017 at 08:38:10PM +0000, Hans de Goede wrote:
> Hi,
> 
> On 19-12-17 20:42, Rodrigo Vivi wrote:
> > 
> > On Tue, Dec 19, 2017 at 07:21:13PM +0000, Hans de Goede wrote:
> > > At least on the Chuwi Vi8 (non pro/plus) the LCD panel will show an image
> > > shifted aprox. 20% to the left (with wraparound) and sometimes also wrong
> > > colors, showing that the panel controller is starting with sampling the
> > > datastream somewhere mid-line. This happens after the first blanking and
> > > re-init of the panel.
> > > 
> > > After looking at drm.debug output I noticed that initially we inherit the
> > > cdclk of 333333 KHz set by the GOP, but after the re-init we picked 266667
> > > KHz, which turns out to be the cause of this problem, a quick hack to hard
> > > code the cdclk to 333333 KHz makes the problem go away.
> > > 
> > > I've tested this on various Bay Trail devices:
> > > -Chuwi Vi8: 800x1280, 333333 at boot, requires 333333 to work properly
> > > -GP-electronic T701: 1024x600, 333333 at boot, works fine with 333333
> > > -PEAQ C1010: 1920x1200, 333333 at boot, works fine with 333333
> > > -PoV mobii-wintab-800w: 800x1280, 333333 at boot, works fine with 333333
> > > -Asus Transformer-T100TA: 1368x768, 320000 at boot, works fine with 320000
> > 
> > and lower clock shift the image in all of them?
> 
> No, just on the Chuwi Vi8, the tests was to make sure the higher clock
> does not create problems elsewhere and I was curious what clock the GOP
> was using on other devices. Also hence the "requires 333333 to work properly"
> vs "works fine with 333333" in the list.
> 
> > > So it seems that the GOP is always using what vlv_calc_cdclk calls
> > > freq_320 as cdclk clock. Also note the comment in vlv_calc_cdclk about
> > > it avoiding 200 Mhz as clock because that causes issues in some cases.
> > > 
> > > This commit extends the "do not use 200 Mhz" workaround with an extra
> > > check to require atleast 320000 KHz (avoiding 266667 KHz) when a DSI
> > > panel is active.
> > > 
> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > > ---
> > >   drivers/gpu/drm/i915/intel_cdclk.c | 8 ++++++++
> > >   1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 9c5ceb98d48f..cfb3f7fb3e1c 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1923,6 +1923,14 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
> > >   	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
> > >   		min_cdclk = max(2 * 96000, min_cdclk);
> > > +	/*
> > > +	 * On Valleyview the GOP always uses 320000/333333 KHz if DSI is used,
> > > +	 * using a lower clock causes causes sync issues with some panels.
> > > +	 */
> > 
> > Afaik (afai-have-noticed) GOP always use the higher freq and higher rates
> > that panels support with no care about saving any power. So I'm not sure if
> > that should be used as any sort of reference for us.
> > 
> > Note I'm not telling to not add the workaround itself...
> 
> So you're suggesting to change the comment into something like this
> instead ?  :
> 
> /*
>  * On Valleyview some DSI panels loose (v|h)sync when the clock is lower
>  * then 320000KHz.
>  */

Yeap... something like that so we don't start counting GOP as the reference in the future ;)

> 
> > 
> > > +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) &&
> > > +	    IS_VALLEYVIEW(dev_priv))
> > > +		min_cdclk = max(320000, min_cdclk);
> > > +
> > >   	if (min_cdclk > dev_priv->max_cdclk_freq) {
> > >   		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
> > >   			      min_cdclk, dev_priv->max_cdclk_freq);
> > > -- 
> > > 2.14.3
> > > 
> 
> Regards,
> 
> Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel