diff mbox

WARNING: CPU: 1 PID: 2846 at drivers/gpu/drm/i915/intel_sdvo.c:1378

Message ID 20130802092610.GJ5004@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Aug. 2, 2013, 9:26 a.m. UTC
On Thu, Aug 01, 2013 at 05:51:31PM -0700, Linus Torvalds wrote:
> This one may have been going on for some time - I haven't updated the
> old Intel Mac Mini in a while.
> 
> And by "not updated" I also mean that it's some really old user-space:
> the machine is running F14, and cannot be updated to anything newer
> without having to reinstall everything because of a stupid small /boot
> partition. So it's not going to happen. But maybe somebody cares about
> the i915 warning anyway?
> 
>                 Linus
> 
> ---
>    WARNING: CPU: 1 PID: 2846 at drivers/gpu/drm/i915/intel_sdvo.c:1378
> intel_sdvo_get_config+0x158/0x160()
>    SDVO pixel multiplier mismatch, port: 0, encoder: 1
>    Modules linked in: snd_hda_codec_idt snd_hda_intel snd_hda_codec snd_hwdep
>    CPU: 1 PID: 2846 Comm: Xorg Not tainted 3.11.0-rc3-00208-gbe1e8d7-dirty #19
>    Hardware name: Apple Computer, Inc. Macmini1,1/Mac-F4208EC8, BIOS
>   MM11.88Z.0055.B03.0604071521 04/07/06
>     00000000 00000000 ef0afa54 c1597bbb c1737ea4 ef0afa84 c10392ca c1737e6c
>     ef0afab0 00000b1e c1737ea4 00000562 c12dfbe8 c12dfbe8 ef0afb14 00000000
>     f697ec00 ef0afa9c c103936e 00000009 ef0afa94 c1737e6c ef0afab0 ef0afadc
>    Call Trace:
>     [<c1597bbb>] dump_stack+0x41/0x56
>     [<c10392ca>] warn_slowpath_common+0x7a/0xa0
>     [<c103936e>] warn_slowpath_fmt+0x2e/0x30
>     [<c12dfbe8>] intel_sdvo_get_config+0x158/0x160
>     [<c12c3220>] check_crtc_state+0x1e0/0xb10
>     [<c12cdc7d>] intel_modeset_check_state+0x29d/0x7c0
>     [<c12dfe5c>] intel_sdvo_dpms+0x5c/0xa0
>     [<c12985de>] drm_mode_obj_set_property_ioctl+0x40e/0x420
>     [<c1298625>] drm_mode_connector_property_set_ioctl+0x35/0x40
>     [<c1289294>] drm_ioctl+0x3e4/0x540
>     [<c10fc1a2>] do_vfs_ioctl+0x72/0x570
>     [<c10fc72f>] SyS_ioctl+0x8f/0xa0
>     [<c159b7fa>] sysenter_do_call+0x12/0x22
>    ---[ end trace 7ce940aff1366d60 ]---

Looks like this could happen when you go to DPMS_OFF. After we've turned
off the the pipe, get_pipe_config() gives a mostly zeroed pipe_config
(including pixel_multiplier), and then in the case of SDVO, we go and
check it against the encoder's idea of pixel_multiplier anyway.

I'm thinking perhaps we shouldn't call get_config for inactive
encoders. How about the following patch?

From 16c4048bfb54e338ce23a7e98af96af2f58b0e0d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Fri, 2 Aug 2013 12:06:47 +0300
Subject: [PATCH] drm/i915: Don't call encoder's get_config unless encoder is
 active

---
 drivers/gpu/drm/i915/intel_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Aug. 4, 2013, 4:52 p.m. UTC | #1
On Fri, Aug 2, 2013 at 11:26 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Aug 01, 2013 at 05:51:31PM -0700, Linus Torvalds wrote:
>> This one may have been going on for some time - I haven't updated the
>> old Intel Mac Mini in a while.
>>
>> And by "not updated" I also mean that it's some really old user-space:
>> the machine is running F14, and cannot be updated to anything newer
>> without having to reinstall everything because of a stupid small /boot
>> partition. So it's not going to happen. But maybe somebody cares about
>> the i915 warning anyway?
>>
>>                 Linus
>>
>> ---
>>    WARNING: CPU: 1 PID: 2846 at drivers/gpu/drm/i915/intel_sdvo.c:1378
>> intel_sdvo_get_config+0x158/0x160()
>>    SDVO pixel multiplier mismatch, port: 0, encoder: 1
>>    Modules linked in: snd_hda_codec_idt snd_hda_intel snd_hda_codec snd_hwdep
>>    CPU: 1 PID: 2846 Comm: Xorg Not tainted 3.11.0-rc3-00208-gbe1e8d7-dirty #19
>>    Hardware name: Apple Computer, Inc. Macmini1,1/Mac-F4208EC8, BIOS
>>   MM11.88Z.0055.B03.0604071521 04/07/06
>>     00000000 00000000 ef0afa54 c1597bbb c1737ea4 ef0afa84 c10392ca c1737e6c
>>     ef0afab0 00000b1e c1737ea4 00000562 c12dfbe8 c12dfbe8 ef0afb14 00000000
>>     f697ec00 ef0afa9c c103936e 00000009 ef0afa94 c1737e6c ef0afab0 ef0afadc
>>    Call Trace:
>>     [<c1597bbb>] dump_stack+0x41/0x56
>>     [<c10392ca>] warn_slowpath_common+0x7a/0xa0
>>     [<c103936e>] warn_slowpath_fmt+0x2e/0x30
>>     [<c12dfbe8>] intel_sdvo_get_config+0x158/0x160
>>     [<c12c3220>] check_crtc_state+0x1e0/0xb10
>>     [<c12cdc7d>] intel_modeset_check_state+0x29d/0x7c0
>>     [<c12dfe5c>] intel_sdvo_dpms+0x5c/0xa0
>>     [<c12985de>] drm_mode_obj_set_property_ioctl+0x40e/0x420
>>     [<c1298625>] drm_mode_connector_property_set_ioctl+0x35/0x40
>>     [<c1289294>] drm_ioctl+0x3e4/0x540
>>     [<c10fc1a2>] do_vfs_ioctl+0x72/0x570
>>     [<c10fc72f>] SyS_ioctl+0x8f/0xa0
>>     [<c159b7fa>] sysenter_do_call+0x12/0x22
>>    ---[ end trace 7ce940aff1366d60 ]---
>
> Looks like this could happen when you go to DPMS_OFF. After we've turned
> off the the pipe, get_pipe_config() gives a mostly zeroed pipe_config
> (including pixel_multiplier), and then in the case of SDVO, we go and
> check it against the encoder's idea of pixel_multiplier anyway.
>
> I'm thinking perhaps we shouldn't call get_config for inactive
> encoders. How about the following patch?

Yeah, patch makes sense. Unfortunately we can't really merge the
->get_hw_state and ->get_config callbacks due to the pipe depencency
:(

Linus, if this does't help please boot with drm.debug=0xe (or set it
at runtime in /sys/modules), reproduce a new backtrace and then attach
the dmesg. That should give us enough context to reconstruct what's
going wrong.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Linus Torvalds Aug. 4, 2013, 6:38 p.m. UTC | #2
On Fri, Aug 2, 2013 at 2:26 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
>
> Looks like this could happen when you go to DPMS_OFF. After we've turned
> off the the pipe, get_pipe_config() gives a mostly zeroed pipe_config
> (including pixel_multiplier), and then in the case of SDVO, we go and
> check it against the encoder's idea of pixel_multiplier anyway.
>
> I'm thinking perhaps we shouldn't call get_config for inactive
> encoders. How about the following patch?

Yup, seems to fix it for that machine.

Thanks,

             Linus
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e1e50df..6396bca 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8549,9 +8549,11 @@  check_crtc_state(struct drm_device *dev)
 
 		list_for_each_entry(encoder, &dev->mode_config.encoder_list,
 				    base.head) {
+			enum pipe pipe;
 			if (encoder->base.crtc != &crtc->base)
 				continue;
-			if (encoder->get_config)
+			if (encoder->get_config &&
+			    encoder->get_hw_state(encoder, &pipe))
 				encoder->get_config(encoder, &pipe_config);
 		}