2013-09-06 21:58:47

by Knut Petersen

[permalink] [raw]
Subject: [BUG kernel 3.11+] i915: pipe state doesn't match

Someone might be interested ... kernel 2e032852245b3dcfe5461d7353e34eb6da095ccf.

[ 0.000000] Linux version 3.11.0-main+ ([email protected]) (gcc version 4.7.2 20130108 [gcc-4_7-branch revision 195012] (SUSE Linux) ) #34 PREEMPT Fri Sep 6 09:46:35 CEST 2013

[ 2.258908] Linux agpgart interface v0.103
[ 2.259082] agpgart-intel 0000:00:00.0: Intel 915GM Chipset
[ 2.259258] agpgart-intel 0000:00:00.0: detected gtt size: 262144K total, 262144K mappable
[ 2.259949] agpgart-intel 0000:00:00.0: detected 8192K stolen memory
[ 2.260466] agpgart-intel 0000:00:00.0: AGP aperture is 256M @ 0xc0000000
[ 2.260608] [drm] Initialized drm 1.1.0 20060810
[ 2.265158] [drm] Memory usable by graphics device = 256M
[ 2.265301] i915 0000:00:02.0: setting latency timer to 64
[ 2.266465] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[ 2.266471] [drm] Driver supports precise vblank timestamp query.
[ 2.267197] [drm:i915_stolen_to_physical] *ERROR* conflict detected with stolen region: [0x7f800000 - 0x80000000]
[ 2.267876] vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[ 2.268246] [drm] Skipping LVDS initialization for AOpen i915GMm-HFS
[ 3.132208] tsc: Refined TSC clocksource calibration: 1199.999 MHz
[ 3.169857] [drm] initialized overlay support
[ 3.651142] [drm:intel_pipe_config_compare] *ERROR* mismatch in adjusted_mode.flags(DRM_MODE_FLAG_NHSYNC) (expected 2, found 0)
[ 3.651221] ------------[ cut here ]------------
[ 3.651237] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/intel_display.c:8746 check_crtc_state+0x6c5/0x6f9()
[ 3.651242] pipe state doesn't match!
[ 3.651246] Modules linked in:
[ 3.651256] CPU: 0 PID: 1 Comm: swapper Not tainted 3.11.0-main+ #34
[ 3.651262] Hardware name: /i915GMm-HFS, BIOS 6.00 PG 11/04/2005
[ 3.651267] 00000000 c07019fb f60798a0 c053b1c2 f60798b8 c012a8f0 c03c578b 00000000
[ 3.651288] f6377000 f636cc48 f60798d0 c012a97a 00000009 f60798c8 c07019fb f60798e4
[ 3.651308] f6079b38 c03c578b c07008c1 0000222a c07019fb f606a4f8 f6376e44 f606a4fc
[ 3.651329] Call Trace:
[ 3.651342] [<c053b1c2>] dump_stack+0x16/0x18
[ 3.651354] [<c012a8f0>] warn_slowpath_common+0x5f/0x76
[ 3.651363] [<c03c578b>] ? check_crtc_state+0x6c5/0x6f9
[ 3.651373] [<c012a97a>] warn_slowpath_fmt+0x2b/0x2f
[ 3.651383] [<c03c578b>] check_crtc_state+0x6c5/0x6f9
[ 3.651411] [<c03cebf6>] intel_modeset_check_state+0x30c/0x57b
[ 3.651422] [<c03cee8b>] intel_set_mode+0x26/0x2f
[ 3.651431] [<c03cfdba>] intel_get_load_detect_pipe+0x2b4/0x308
[ 3.651447] [<c03e9b9f>] intel_tv_detect+0x108/0x444
[ 3.651466] [<c038dd86>] drm_helper_probe_single_connector_modes+0xa0/0x270
[ 3.651477] [<c038b88a>] drm_fb_helper_probe_connector_modes+0x39/0x4c
[ 3.651487] [<c038cd6f>] drm_fb_helper_initial_config+0x143/0x3ac
[ 3.651497] [<c0540be0>] ? _raw_spin_unlock_irqrestore+0x38/0x5b
[ 3.651506] [<c0540bec>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.651516] [<c0540bec>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.651527] [<c03e94ef>] intel_fbdev_initial_config+0x1e/0x20
[ 3.651536] [<c03a6efd>] i915_driver_load+0xb48/0xd23
[ 3.651551] [<c03976b6>] drm_get_pci_dev+0x172/0x266
[ 3.651560] [<c0540bec>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.651572] [<c03a41d5>] i915_pci_probe+0x46/0x4f
[ 3.651582] [<c0304fb3>] pci_device_probe+0x5e/0x96
[ 3.651594] [<c0401ee5>] driver_probe_device+0x8c/0x186
[ 3.651604] [<c0402037>] __driver_attach+0x58/0x76
[ 3.651614] [<c0400892>] bus_for_each_dev+0x43/0x6d
[ 3.651624] [<c0401b0b>] driver_attach+0x19/0x1b
[ 3.651633] [<c0401fdf>] ? driver_probe_device+0x186/0x186
[ 3.651642] [<c0401746>] bus_add_driver+0xcc/0x1f7
[ 3.651652] [<c0402554>] driver_register+0x73/0xa5
[ 3.651661] [<c0305096>] __pci_register_driver+0x4a/0x4d
[ 3.651673] [<c07fb56a>] ? ftrace_define_fields_drm_vblank_event+0x45/0x45
[ 3.651682] [<c0397817>] drm_pci_init+0x6d/0xc5
[ 3.651692] [<c07fb56a>] ? ftrace_define_fields_drm_vblank_event+0x45/0x45
[ 3.651701] [<c07fb5c8>] i915_init+0x5e/0x60
[ 3.651710] [<c01003b5>] do_one_initcall+0x6f/0xea
[ 3.651722] [<c07d2428>] ? repair_env_string+0x12/0x51
[ 3.651731] [<c07d2400>] ? do_early_param+0x5f/0x75
[ 3.651741] [<c014147f>] ? parse_args+0x16b/0x209
[ 3.651752] [<c07d29c2>] kernel_init_freeable+0xce/0x153
[ 3.651762] [<c053556a>] kernel_init+0xd/0xb9
[ 3.651771] [<c0546277>] ret_from_kernel_thread+0x1b/0x28
[ 3.651779] [<c053555d>] ? rest_init+0xa5/0xa5
[ 3.651958] ---[ end trace deefedea51430d57 ]---
[ 3.824510] fbcon: inteldrmfb (fb0) is primary device
[ 3.943377] Console: switching to colour frame buffer device 160x64
[ 3.957788] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[ 3.957795] i915 0000:00:02.0: registered panic notifier
[ 3.957864] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0

cu,
Knut


2013-09-07 12:52:20

by Knut Petersen

[permalink] [raw]
Subject: Re: [BUG kernel 3.11+] i915: pipe state doesn't match

On 07.09.2013 06:19, Hillf Danton wrote:
> If it works after reverting commit,
>
> 3eaba51cd399f5362a9fd9ebd5fb8b625b454271
> drm/i915: Don't call encoder's get_config unless encoder is active

Hmm, after reverting 3eaba51cd399f5362a9fd9ebd5fb8b625b454271 I still see:

[ 2.259897] Linux agpgart interface v0.103
[ 2.260098] agpgart-intel 0000:00:00.0: Intel 915GM Chipset
[ 2.260266] agpgart-intel 0000:00:00.0: detected gtt size: 262144K total, 262144K mappable
[ 2.260975] agpgart-intel 0000:00:00.0: detected 8192K stolen memory
[ 2.261441] agpgart-intel 0000:00:00.0: AGP aperture is 256M @ 0xc0000000
[ 2.261585] [drm] Initialized drm 1.1.0 20060810
[ 2.266079] [drm] Memory usable by graphics device = 256M
[ 2.266223] i915 0000:00:02.0: setting latency timer to 64
[ 2.267393] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010).
[ 2.267399] [drm] Driver supports precise vblank timestamp query.
[ 2.268255] [drm:i915_stolen_to_physical] *ERROR* conflict detected with stolen region: [0x7f800000 - 0x80000000]
[ 2.268935] vgaarb: device changed decodes: PCI:0000:00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[ 2.269304] [drm] Skipping LVDS initialization for AOpen i915GMm-HFS
[ 3.136204] tsc: Refined TSC clocksource calibration: 1199.999 MHz
[ 3.171679] [drm] initialized overlay support
[ 3.655174] [drm:intel_pipe_config_compare] *ERROR* mismatch in adjusted_mode.flags(DRM_MODE_FLAG_NHSYNC) (expected 2, found 0)
[ 3.655252] ------------[ cut here ]------------
[ 3.655268] WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/i915/intel_display.c:8744 check_crtc_state+0x6b3/0x6e7()
[ 3.655273] pipe state doesn't match!
[ 3.655278] Modules linked in:
[ 3.655288] CPU: 0 PID: 1 Comm: swapper Not tainted 3.11.0-main+ #36
[ 3.655293] Hardware name: /i915GMm-HFS, BIOS 6.00 PG 11/04/2005
[ 3.655299] 00000000 c0701a59 f60798a4 c053b528 f60798bc c012a930 c03c5b1d 00000000
[ 3.655320] f6375000 f636cc48 f60798d4 c012a9ba 00000009 f60798cc c0701a59 f60798e8
[ 3.655341] f6079b38 c03c5b1d c070091f 00002228 c0701a59 f623c024 f6374e44 f6079964
[ 3.655362] Call Trace:
[ 3.655375] [<c053b528>] dump_stack+0x16/0x18
[ 3.655387] [<c012a930>] warn_slowpath_common+0x5f/0x76
[ 3.655397] [<c03c5b1d>] ? check_crtc_state+0x6b3/0x6e7
[ 3.655406] [<c012a9ba>] warn_slowpath_fmt+0x2b/0x2f
[ 3.655417] [<c03c5b1d>] check_crtc_state+0x6b3/0x6e7
[ 3.655445] [<c03cef88>] intel_modeset_check_state+0x30c/0x57b
[ 3.655456] [<c03cf21d>] intel_set_mode+0x26/0x2f
[ 3.655465] [<c03d014c>] intel_get_load_detect_pipe+0x2b4/0x308
[ 3.655480] [<c03e9f33>] intel_tv_detect+0x108/0x444
[ 3.655500] [<c038e12a>] drm_helper_probe_single_connector_modes+0xa0/0x270
[ 3.655511] [<c038bc2e>] drm_fb_helper_probe_connector_modes+0x39/0x4c
[ 3.655522] [<c038d113>] drm_fb_helper_initial_config+0x143/0x3ac
[ 3.655532] [<c0540f48>] ? _raw_spin_unlock_irqrestore+0x38/0x5b
[ 3.655541] [<c0540f54>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.655551] [<c0540f54>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.655562] [<c03e9883>] intel_fbdev_initial_config+0x1e/0x20
[ 3.655571] [<c03a72a1>] i915_driver_load+0xb48/0xd23
[ 3.655587] [<c0397a5a>] drm_get_pci_dev+0x172/0x266
[ 3.655596] [<c0540f54>] ? _raw_spin_unlock_irqrestore+0x44/0x5b
[ 3.655608] [<c03a4579>] i915_pci_probe+0x46/0x4f
[ 3.655618] [<c0305243>] pci_device_probe+0x5e/0x96
[ 3.655631] [<c0402279>] driver_probe_device+0x8c/0x186
[ 3.655641] [<c04023cb>] __driver_attach+0x58/0x76
[ 3.655651] [<c0400c26>] bus_for_each_dev+0x43/0x6d
[ 3.655660] [<c0401e9f>] driver_attach+0x19/0x1b
[ 3.655670] [<c0402373>] ? driver_probe_device+0x186/0x186
[ 3.655679] [<c0401ada>] bus_add_driver+0xcc/0x1f7
[ 3.655689] [<c04028e8>] driver_register+0x73/0xa5
[ 3.655699] [<c0305326>] __pci_register_driver+0x4a/0x4d
[ 3.655710] [<c07fb56a>] ? ftrace_define_fields_drm_vblank_event+0x45/0x45
[ 3.655720] [<c0397bbb>] drm_pci_init+0x6d/0xc5
[ 3.655729] [<c07fb56a>] ? ftrace_define_fields_drm_vblank_event+0x45/0x45
[ 3.655739] [<c07fb5c8>] i915_init+0x5e/0x60
[ 3.655748] [<c01003b5>] do_one_initcall+0x6f/0xea
[ 3.655760] [<c07d2428>] ? repair_env_string+0x12/0x51
[ 3.655769] [<c07d2400>] ? do_early_param+0x5f/0x75
[ 3.655779] [<c01414bf>] ? parse_args+0x16b/0x209
[ 3.655790] [<c07d29c2>] kernel_init_freeable+0xce/0x153
[ 3.655800] [<c05358ca>] kernel_init+0xd/0xb9
[ 3.655810] [<c05465f7>] ret_from_kernel_thread+0x1b/0x28
[ 3.655818] [<c05358bd>] ? rest_init+0xa5/0xa5
[ 3.655998] ---[ end trace 9f305b1183e512f6 ]---
[ 3.829415] fbcon: inteldrmfb (fb0) is primary device
[ 3.943020] Console: switching to colour frame buffer device 160x64
[ 3.957357] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[ 3.957364] i915 0000:00:02.0: registered panic notifier
[ 3.957434] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0

cu,
Knut

2013-09-07 16:54:51

by Daniel Vetter

[permalink] [raw]
Subject: Re: [Intel-gfx] [BUG kernel 3.11+] i915: pipe state doesn't match

On Fri, Sep 6, 2013 at 11:58 PM, Knut Petersen
<[email protected]> wrote:
> Someone might be interested ... kernel
> 2e032852245b3dcfe5461d7353e34eb6da095ccf.
>
> [ 0.000000] Linux version 3.11.0-main+ ([email protected]) (gcc
> version 4.7.2 20130108 [gcc-4_7-branch revision 195012] (SUSE Linux) ) #34
> PREEMPT Fri Sep 6 09:46:35 CEST 2013


http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=4c6df4b4ca1b26f4532d403b544f649a1c801fd1

could be of interest for you. If that doesn't help please boot with
drm.debug=0xe and attach the complete dmesg.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-09-09 10:08:17

by Knut Petersen

[permalink] [raw]
Subject: Re: [Intel-gfx] [BUG kernel 3.11+] i915: pipe state doesn't match

On 07.09.2013 18:54, Daniel Vetter wrote:
> On Fri, Sep 6, 2013 at 11:58 PM, Knut Petersen
> <[email protected]> wrote:
>> Someone might be interested ... kernel
>> 2e032852245b3dcfe5461d7353e34eb6da095ccf.
>>
>> [ 0.000000] Linux version 3.11.0-main+ ([email protected]) (gcc
>> version 4.7.2 20130108 [gcc-4_7-branch revision 195012] (SUSE Linux) ) #34
>> PREEMPT Fri Sep 6 09:46:35 CEST 2013
>
> http://cgit.freedesktop.org/~danvet/drm-intel/commit/?id=4c6df4b4ca1b26f4532d403b544f649a1c801fd1

That does not help, so dmesg attached
> could be of interest for you. If that doesn't help please boot with
> drm.debug=0xe and attach the complete dmesg.
>
> Thanks, Daniel


Attachments:
dmesg (103.45 kB)

2013-09-10 05:20:37

by Knut Petersen

[permalink] [raw]
Subject: Re: [Intel-gfx] [BUG kernel 3.11+] i915: pipe state doesn't match

Addendum: Attached find a log of a suspend / resume cycle, debug level 0xe.
Kernel 26b0332e30c7f93e780aaa054bd84e3437f84354 with added stolen memory patches.

cu,
Knut


Attachments:
dmesg (60.24 kB)

2013-09-10 08:02:48

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

Instead of just a flag bit for each of the positive/negative sync
modes drm actually uses a separate flag for each ... This upsets the
modeset checker since the adjusted mode filled out at modeset time
doesn't match the one reconstructed at check time (since the
->get_config callback already gets this right).

Reported-by: Knut Petersen <[email protected]>
Cc: Knut Petersen <[email protected]>
References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
Signed-off-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/intel_sdvo.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 85037b9..5033c74 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -866,8 +866,12 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
mode->flags |= DRM_MODE_FLAG_INTERLACE;
if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
mode->flags |= DRM_MODE_FLAG_PHSYNC;
+ else
+ mode->flags |= DRM_MODE_FLAG_NHSYNC;
if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
mode->flags |= DRM_MODE_FLAG_PVSYNC;
+ else
+ mode->flags |= DRM_MODE_FLAG_NVSYNC;
}

static bool intel_sdvo_check_supp_encode(struct intel_sdvo *intel_sdvo)
--
1.8.4.rc3

2013-09-10 08:25:44

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On Tue, Sep 10, 2013 at 10:02:48AM +0200, Daniel Vetter wrote:
> Instead of just a flag bit for each of the positive/negative sync
> modes drm actually uses a separate flag for each ... This upsets the
> modeset checker since the adjusted mode filled out at modeset time
> doesn't match the one reconstructed at check time (since the
> ->get_config callback already gets this right).
>
> Reported-by: Knut Petersen <[email protected]>
> Cc: Knut Petersen <[email protected]>
> References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
> Signed-off-by: Daniel Vetter <[email protected]>

Reviewed-by: Ville Syrj?l? <[email protected]>

> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 85037b9..5033c74 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -866,8 +866,12 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
> mode->flags |= DRM_MODE_FLAG_INTERLACE;
> if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NHSYNC;
> if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NVSYNC;
> }
>
> static bool intel_sdvo_check_supp_encode(struct intel_sdvo *intel_sdvo)
> --
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrj?l?
Intel OTC

2013-09-10 08:41:54

by Knut Petersen

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On 10.09.2013 10:02, Daniel Vetter wrote:
> Instead of just a flag bit for each of the positive/negative sync
> modes drm actually uses a separate flag for each ... This upsets the
> modeset checker since the adjusted mode filled out at modeset time
> doesn't match the one reconstructed at check time (since the
> ->get_config callback already gets this right).
>
> Reported-by: Knut Petersen <[email protected]>
> Cc: Knut Petersen <[email protected]>
> References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded

Thanks for the patch, but it does _not_ cure the problem, see attached boot dmesg.

cu,
Knut


> Signed-off-by: Daniel Vetter <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 85037b9..5033c74 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -866,8 +866,12 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
> mode->flags |= DRM_MODE_FLAG_INTERLACE;
> if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NHSYNC;
> if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NVSYNC;
> }
>
> static bool intel_sdvo_check_supp_encode(struct intel_sdvo *intel_sdvo)


Attachments:
dmesg (103.75 kB)

2013-09-10 09:28:26

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On Tue, Sep 10, 2013 at 10:41:33AM +0200, Knut Petersen wrote:
> On 10.09.2013 10:02, Daniel Vetter wrote:
> >Instead of just a flag bit for each of the positive/negative sync
> >modes drm actually uses a separate flag for each ... This upsets the
> >modeset checker since the adjusted mode filled out at modeset time
> >doesn't match the one reconstructed at check time (since the
> >->get_config callback already gets this right).
> >
> >Reported-by: Knut Petersen <[email protected]>
> >Cc: Knut Petersen <[email protected]>
> >References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
>
> Thanks for the patch, but it does _not_ cure the problem, see attached boot dmesg.

Oh, it fixed the first mismatch afaict, but I've missed that the tv load
detect also causes fun ;-) Working on a 2nd patch ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-09-10 09:38:37

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On Tue, Sep 10, 2013 at 10:02:48AM +0200, Daniel Vetter wrote:
> Instead of just a flag bit for each of the positive/negative sync
> modes drm actually uses a separate flag for each ... This upsets the
> modeset checker since the adjusted mode filled out at modeset time
> doesn't match the one reconstructed at check time (since the
> ->get_config callback already gets this right).
>
> Reported-by: Knut Petersen <[email protected]>
> Cc: Knut Petersen <[email protected]>
> References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
> Signed-off-by: Daniel Vetter <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_sdvo.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 85037b9..5033c74 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -866,8 +866,12 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
> mode->flags |= DRM_MODE_FLAG_INTERLACE;
> if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NHSYNC;
> if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> mode->flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + mode->flags |= DRM_MODE_FLAG_NVSYNC;

Actually it seems we don't recreate the mode from scratch here, so we
should also clear the negative sync flags (and the interlace flag too)
before we set them.

> }
>
> static bool intel_sdvo_check_supp_encode(struct intel_sdvo *intel_sdvo)
> --
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrj?l?
Intel OTC

2013-09-10 09:42:31

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On Tue, Sep 10, 2013 at 12:38:24PM +0300, Ville Syrj?l? wrote:
> On Tue, Sep 10, 2013 at 10:02:48AM +0200, Daniel Vetter wrote:
> > Instead of just a flag bit for each of the positive/negative sync
> > modes drm actually uses a separate flag for each ... This upsets the
> > modeset checker since the adjusted mode filled out at modeset time
> > doesn't match the one reconstructed at check time (since the
> > ->get_config callback already gets this right).
> >
> > Reported-by: Knut Petersen <[email protected]>
> > Cc: Knut Petersen <[email protected]>
> > References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
> > Signed-off-by: Daniel Vetter <[email protected]>
> > ---
> > drivers/gpu/drm/i915/intel_sdvo.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 85037b9..5033c74 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -866,8 +866,12 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
> > mode->flags |= DRM_MODE_FLAG_INTERLACE;
> > if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
> > mode->flags |= DRM_MODE_FLAG_PHSYNC;
> > + else
> > + mode->flags |= DRM_MODE_FLAG_NHSYNC;
> > if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
> > mode->flags |= DRM_MODE_FLAG_PVSYNC;
> > + else
> > + mode->flags |= DRM_MODE_FLAG_NVSYNC;
>
> Actually it seems we don't recreate the mode from scratch here, so we
> should also clear the negative sync flags (and the interlace flag too)
> before we set them.

... and we seem to be missing a drm_mode_set_crtcinfo() call too.

>
> > }
> >
> > static bool intel_sdvo_check_supp_encode(struct intel_sdvo *intel_sdvo)
> > --
> > 1.8.4.rc3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrj?l?
> Intel OTC

--
Ville Syrj?l?
Intel OTC

2013-09-10 09:44:24

by Daniel Vetter

[permalink] [raw]
Subject: [PATCH] drm/i915/tv: clear adjusted_mode.flags

The native TV encoder has it's own flags to adjust sync modes and
enabled interlaced modes which are totally irrelevant for the adjusted
mode. This worked out nicely since the input modes used by both the
load detect code and reported in the ->get_modes callbacks all have no
flags set, and we also don't fill out any of them in the ->get_config
callback.

This changed with the additional sanitation done with

commit 2960bc9cceecb5d556ce1c07656a6609e2f7e8b0
Author: Imre Deak <[email protected]>
Date: Tue Jul 30 13:36:32 2013 +0300

drm/i915: make user mode sync polarity setting explicit

sinc now the "no flags at all" state wouldn't fit through core code
any more. So fix this up again by explicitly clearing the flags in the
->compute_config callback.

Aside: We have zero checking in place to make sure that the requested
mode is indeed the right input mode we want for the selected TV mode.
So we'll happily fall over if userspace tries to pull us. But that's
definitely work for a different patch series. So just add a FIXME
comment for now.

Reported-by: Knut Petersen <[email protected]>
Cc: Knut Petersen <[email protected]>
Cc: Imre Deak <[email protected]>
Cc: Chris Wilson <[email protected]>
Signed-off-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/i915/intel_tv.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index f2c6d79..dd6f84b 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -916,6 +916,14 @@ intel_tv_compute_config(struct intel_encoder *encoder,
DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
pipe_config->pipe_bpp = 8*3;

+ /* TV has it's own notion of sync and other mode flags, so clear them. */
+ pipe_config->adjusted_mode.flags = 0;
+
+ /*
+ * FIXME: We don't check whether the input mode is actually what we want
+ * or whether userspace is doing something stupid.
+ */
+
return true;
}

--
1.8.4.rc3

2013-09-10 10:11:12

by Knut Petersen

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/tv: clear adjusted_mode.flags

On 10.09.2013 11:44, Daniel Vetter wrote:

Thanks. Combined with your two patches and Jesse Barnes' patch
"add early quirk for reserving Intel graphics stolen memory v5",
current linux master branch works fine on i915GM hardware now.

cu,
Knut

2013-09-10 10:34:11

by Daniel Vetter

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/sdvo: Fully translate sync flags in the dtd->mode conversion

On Tue, Sep 10, 2013 at 11:24:52AM +0300, Ville Syrj?l? wrote:
> On Tue, Sep 10, 2013 at 10:02:48AM +0200, Daniel Vetter wrote:
> > Instead of just a flag bit for each of the positive/negative sync
> > modes drm actually uses a separate flag for each ... This upsets the
> > modeset checker since the adjusted mode filled out at modeset time
> > doesn't match the one reconstructed at check time (since the
> > ->get_config callback already gets this right).
> >
> > Reported-by: Knut Petersen <[email protected]>
> > Cc: Knut Petersen <[email protected]>
> > References: http://www.gossamer-threads.com/lists/linux/kernel/1778688?do=post_view_threaded
> > Signed-off-by: Daniel Vetter <[email protected]>
>
> Reviewed-by: Ville Syrj?l? <[email protected]>

Picked up for -fixes, thanks for the review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-09-24 05:52:20

by Knut Petersen

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/tv: clear adjusted_mode.flags

Hi Daniel!

This patch is definitely needed for 3.12, but it is still missing in 3.12-rc2 ...

cu,
Knut

On 10.09.2013 11:44, Daniel Vetter wrote:
> The native TV encoder has it's own flags to adjust sync modes and
> enabled interlaced modes which are totally irrelevant for the adjusted
> mode. This worked out nicely since the input modes used by both the
> load detect code and reported in the ->get_modes callbacks all have no
> flags set, and we also don't fill out any of them in the ->get_config
> callback.
>
> This changed with the additional sanitation done with
>
> commit 2960bc9cceecb5d556ce1c07656a6609e2f7e8b0
> Author: Imre Deak <[email protected]>
> Date: Tue Jul 30 13:36:32 2013 +0300
>
> drm/i915: make user mode sync polarity setting explicit
>
> sinc now the "no flags at all" state wouldn't fit through core code
> any more. So fix this up again by explicitly clearing the flags in the
> ->compute_config callback.
>
> Aside: We have zero checking in place to make sure that the requested
> mode is indeed the right input mode we want for the selected TV mode.
> So we'll happily fall over if userspace tries to pull us. But that's
> definitely work for a different patch series. So just add a FIXME
> comment for now.
>
> Reported-by: Knut Petersen <[email protected]>
> Cc: Knut Petersen <[email protected]>
> Cc: Imre Deak <[email protected]>
> Cc: Chris Wilson <[email protected]>
> Signed-off-by: Daniel Vetter <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_tv.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index f2c6d79..dd6f84b 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -916,6 +916,14 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
> pipe_config->pipe_bpp = 8*3;
>
> + /* TV has it's own notion of sync and other mode flags, so clear them. */
> + pipe_config->adjusted_mode.flags = 0;
> +
> + /*
> + * FIXME: We don't check whether the input mode is actually what we want
> + * or whether userspace is doing something stupid.
> + */
> +
> return true;
> }
>

2013-09-24 08:00:57

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915/tv: clear adjusted_mode.flags

On Tue, Sep 24, 2013 at 07:52:06AM +0200, Knut Petersen wrote:
> Hi Daniel!
>
> This patch is definitely needed for 3.12, but it is still missing in 3.12-rc2 ...

Oops, that one fell through the cracks. Applied to -fixes, thanks for
poking.
-Daniel

>
> cu,
> Knut
>
> On 10.09.2013 11:44, Daniel Vetter wrote:
> >The native TV encoder has it's own flags to adjust sync modes and
> >enabled interlaced modes which are totally irrelevant for the adjusted
> >mode. This worked out nicely since the input modes used by both the
> >load detect code and reported in the ->get_modes callbacks all have no
> >flags set, and we also don't fill out any of them in the ->get_config
> >callback.
> >
> >This changed with the additional sanitation done with
> >
> >commit 2960bc9cceecb5d556ce1c07656a6609e2f7e8b0
> >Author: Imre Deak <[email protected]>
> >Date: Tue Jul 30 13:36:32 2013 +0300
> >
> > drm/i915: make user mode sync polarity setting explicit
> >
> >sinc now the "no flags at all" state wouldn't fit through core code
> >any more. So fix this up again by explicitly clearing the flags in the
> >->compute_config callback.
> >
> >Aside: We have zero checking in place to make sure that the requested
> >mode is indeed the right input mode we want for the selected TV mode.
> >So we'll happily fall over if userspace tries to pull us. But that's
> >definitely work for a different patch series. So just add a FIXME
> >comment for now.
> >
> >Reported-by: Knut Petersen <[email protected]>
> >Cc: Knut Petersen <[email protected]>
> >Cc: Imre Deak <[email protected]>
> >Cc: Chris Wilson <[email protected]>
> >Signed-off-by: Daniel Vetter <[email protected]>
> >---
> > drivers/gpu/drm/i915/intel_tv.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> >index f2c6d79..dd6f84b 100644
> >--- a/drivers/gpu/drm/i915/intel_tv.c
> >+++ b/drivers/gpu/drm/i915/intel_tv.c
> >@@ -916,6 +916,14 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> > DRM_DEBUG_KMS("forcing bpc to 8 for TV\n");
> > pipe_config->pipe_bpp = 8*3;
> >+ /* TV has it's own notion of sync and other mode flags, so clear them. */
> >+ pipe_config->adjusted_mode.flags = 0;
> >+
> >+ /*
> >+ * FIXME: We don't check whether the input mode is actually what we want
> >+ * or whether userspace is doing something stupid.
> >+ */
> >+
> > return true;
> > }
>

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch