diff mbox series

[v3,6/7] drm: Validate encoder->possible_crtcs

Message ID 20200211162208.16224-7-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm: Try to fix encoder possible_clones/crtc | expand

Commit Message

Ville Syrjälä Feb. 11, 2020, 4:22 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

WARN if the encoder possible_crtcs is effectively empty or contains
bits for non-existing crtcs.

v2: Move to drm_mode_config_validate() (Daniel)
    Make the docs say we WARN when this is wrong (Daniel)
    Extract full_crtc_mask()

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_mode_config.c | 27 ++++++++++++++++++++++++++-
 include/drm/drm_encoder.h         |  2 +-
 2 files changed, 27 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Feb. 11, 2020, 5:04 p.m. UTC | #1
On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> WARN if the encoder possible_crtcs is effectively empty or contains
> bits for non-existing crtcs.
> 
> v2: Move to drm_mode_config_validate() (Daniel)
>     Make the docs say we WARN when this is wrong (Daniel)
>     Extract full_crtc_mask()
> 
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

When pushing the fixup needs to be applied before the validation patch
here, because we don't want to anger the bisect gods.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I think with the fixup we should be good enough with the existing nonsense
in drivers. Fingers crossed.
-Daniel


> ---
>  drivers/gpu/drm/drm_mode_config.c | 27 ++++++++++++++++++++++++++-
>  include/drm/drm_encoder.h         |  2 +-
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index afc91447293a..4c1b350ddb95 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -581,6 +581,29 @@ static void validate_encoder_possible_clones(struct drm_encoder *encoder)
>  	     encoder->possible_clones, encoder_mask);
>  }
>  
> +static u32 full_crtc_mask(struct drm_device *dev)
> +{
> +	struct drm_crtc *crtc;
> +	u32 crtc_mask = 0;
> +
> +	drm_for_each_crtc(crtc, dev)
> +		crtc_mask |= drm_crtc_mask(crtc);
> +
> +	return crtc_mask;
> +}
> +
> +static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
> +{
> +	u32 crtc_mask = full_crtc_mask(encoder->dev);
> +
> +	WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> +	     (encoder->possible_crtcs & ~crtc_mask) != 0,
> +	     "Bogus possible_crtcs: "
> +	     "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> +	     encoder->base.id, encoder->name,
> +	     encoder->possible_crtcs, crtc_mask);
> +}
> +
>  void drm_mode_config_validate(struct drm_device *dev)
>  {
>  	struct drm_encoder *encoder;
> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct drm_device *dev)
>  	drm_for_each_encoder(encoder, dev)
>  		fixup_encoder_possible_clones(encoder);
>  
> -	drm_for_each_encoder(encoder, dev)
> +	drm_for_each_encoder(encoder, dev) {
>  		validate_encoder_possible_clones(encoder);
> +		validate_encoder_possible_crtcs(encoder);
> +	}
>  }
> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> index 3741963b9587..b236269f41ac 100644
> --- a/include/drm/drm_encoder.h
> +++ b/include/drm/drm_encoder.h
> @@ -142,7 +142,7 @@ struct drm_encoder {
>  	 * the bits for all &drm_crtc objects this encoder can be connected to
>  	 * before calling drm_dev_register().
>  	 *
> -	 * In reality almost every driver gets this wrong.
> +	 * You will get a WARN if you get this wrong in the driver.
>  	 *
>  	 * Note that since CRTC objects can't be hotplugged the assigned indices
>  	 * are stable and hence known before registering all objects.
> -- 
> 2.24.1
>
Jan Kiszka Sept. 6, 2020, 11:19 a.m. UTC | #2
On 11.02.20 18:04, Daniel Vetter wrote:
> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> WARN if the encoder possible_crtcs is effectively empty or contains
>> bits for non-existing crtcs.
>>
>> v2: Move to drm_mode_config_validate() (Daniel)
>>     Make the docs say we WARN when this is wrong (Daniel)
>>     Extract full_crtc_mask()
>>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When pushing the fixup needs to be applied before the validation patch
> here, because we don't want to anger the bisect gods.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> I think with the fixup we should be good enough with the existing nonsense
> in drivers. Fingers crossed.
> -Daniel
> 
> 
>> ---
>>  drivers/gpu/drm/drm_mode_config.c | 27 ++++++++++++++++++++++++++-
>>  include/drm/drm_encoder.h         |  2 +-
>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
>> index afc91447293a..4c1b350ddb95 100644
>> --- a/drivers/gpu/drm/drm_mode_config.c
>> +++ b/drivers/gpu/drm/drm_mode_config.c
>> @@ -581,6 +581,29 @@ static void validate_encoder_possible_clones(struct drm_encoder *encoder)
>>  	     encoder->possible_clones, encoder_mask);
>>  }
>>  
>> +static u32 full_crtc_mask(struct drm_device *dev)
>> +{
>> +	struct drm_crtc *crtc;
>> +	u32 crtc_mask = 0;
>> +
>> +	drm_for_each_crtc(crtc, dev)
>> +		crtc_mask |= drm_crtc_mask(crtc);
>> +
>> +	return crtc_mask;
>> +}
>> +
>> +static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
>> +{
>> +	u32 crtc_mask = full_crtc_mask(encoder->dev);
>> +
>> +	WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
>> +	     (encoder->possible_crtcs & ~crtc_mask) != 0,
>> +	     "Bogus possible_crtcs: "
>> +	     "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
>> +	     encoder->base.id, encoder->name,
>> +	     encoder->possible_crtcs, crtc_mask);
>> +}
>> +
>>  void drm_mode_config_validate(struct drm_device *dev)
>>  {
>>  	struct drm_encoder *encoder;
>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct drm_device *dev)
>>  	drm_for_each_encoder(encoder, dev)
>>  		fixup_encoder_possible_clones(encoder);
>>  
>> -	drm_for_each_encoder(encoder, dev)
>> +	drm_for_each_encoder(encoder, dev) {
>>  		validate_encoder_possible_clones(encoder);
>> +		validate_encoder_possible_crtcs(encoder);
>> +	}
>>  }
>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
>> index 3741963b9587..b236269f41ac 100644
>> --- a/include/drm/drm_encoder.h
>> +++ b/include/drm/drm_encoder.h
>> @@ -142,7 +142,7 @@ struct drm_encoder {
>>  	 * the bits for all &drm_crtc objects this encoder can be connected to
>>  	 * before calling drm_dev_register().
>>  	 *
>> -	 * In reality almost every driver gets this wrong.
>> +	 * You will get a WARN if you get this wrong in the driver.
>>  	 *
>>  	 * Note that since CRTC objects can't be hotplugged the assigned indices
>>  	 * are stable and hence known before registering all objects.
>> -- 
>> 2.24.1
>>
> 

Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):

[   14.033246] ------------[ cut here ]------------
[   14.033248] Bogus possible_crtcs: [ENCODER:65:TMDS-65] possible_crtcs=0xf (full crtc mask=0x7)
[   14.033279] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033279] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
[   14.033306]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
[   14.033310] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G            E     5.9.0-rc3+ #2
[   14.033311] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
[   14.033324] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033327] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
[   14.033328] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
[   14.033329] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
[   14.033330] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
[   14.033331] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
[   14.033331] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf8b800
[   14.033332] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
[   14.033333] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
[   14.033334] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   14.033335] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
[   14.033335] Call Trace:
[   14.033350]  drm_dev_register+0x117/0x1e0 [drm]
[   14.033423]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
[   14.033428]  local_pci_probe+0x42/0x90
[   14.033430]  pci_device_probe+0x108/0x1c0
[   14.033433]  really_probe+0xef/0x4a0
[   14.033435]  driver_probe_device+0xde/0x150
[   14.033436]  device_driver_attach+0x4f/0x60
[   14.033438]  __driver_attach+0x9a/0x140
[   14.033439]  ? device_driver_attach+0x60/0x60
[   14.033441]  bus_for_each_dev+0x76/0xc0
[   14.033443]  ? klist_add_tail+0x3b/0x70
[   14.033445]  bus_add_driver+0x144/0x220
[   14.033446]  ? 0xffffffffc0949000
[   14.033447]  driver_register+0x5b/0xf0
[   14.033448]  ? 0xffffffffc0949000
[   14.033451]  do_one_initcall+0x46/0x1f4
[   14.033454]  ? _cond_resched+0x15/0x40
[   14.033456]  ? kmem_cache_alloc_trace+0x40/0x440
[   14.033459]  ? do_init_module+0x22/0x213
[   14.033460]  do_init_module+0x5b/0x213
[   14.033462]  load_module+0x258c/0x2d30
[   14.033465]  ? __kernel_read+0xf5/0x160
[   14.033467]  ? __do_sys_finit_module+0xe9/0x110
[   14.033468]  __do_sys_finit_module+0xe9/0x110
[   14.033471]  do_syscall_64+0x33/0x80
[   14.033473]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   14.033474] RIP: 0033:0x7feacf1bef59
[   14.033477] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
[   14.033477] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   14.033478] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
[   14.033479] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
[   14.033479] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
[   14.033480] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
[   14.033480] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
[   14.033482] ---[ end trace 16aeaa08847a13d8 ]---
[   14.033483] ------------[ cut here ]------------
[   14.033484] Bogus possible_crtcs: [ENCODER:69:TMDS-69] possible_crtcs=0xf (full crtc mask=0x7)
[   14.033507] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033507] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
[   14.033522]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
[   14.033524] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E     5.9.0-rc3+ #2
[   14.033525] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
[   14.033538] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033539] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
[   14.033540] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
[   14.033540] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
[   14.033541] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
[   14.033542] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
[   14.033542] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf89800
[   14.033543] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
[   14.033544] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
[   14.033544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   14.033545] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
[   14.033545] Call Trace:
[   14.033557]  drm_dev_register+0x117/0x1e0 [drm]
[   14.033625]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
[   14.033627]  local_pci_probe+0x42/0x90
[   14.033629]  pci_device_probe+0x108/0x1c0
[   14.033630]  really_probe+0xef/0x4a0
[   14.033632]  driver_probe_device+0xde/0x150
[   14.033633]  device_driver_attach+0x4f/0x60
[   14.033634]  __driver_attach+0x9a/0x140
[   14.033635]  ? device_driver_attach+0x60/0x60
[   14.033636]  bus_for_each_dev+0x76/0xc0
[   14.033638]  ? klist_add_tail+0x3b/0x70
[   14.033639]  bus_add_driver+0x144/0x220
[   14.033640]  ? 0xffffffffc0949000
[   14.033641]  driver_register+0x5b/0xf0
[   14.033642]  ? 0xffffffffc0949000
[   14.033643]  do_one_initcall+0x46/0x1f4
[   14.033645]  ? _cond_resched+0x15/0x40
[   14.033646]  ? kmem_cache_alloc_trace+0x40/0x440
[   14.033648]  ? do_init_module+0x22/0x213
[   14.033649]  do_init_module+0x5b/0x213
[   14.033650]  load_module+0x258c/0x2d30
[   14.033652]  ? __kernel_read+0xf5/0x160
[   14.033654]  ? __do_sys_finit_module+0xe9/0x110
[   14.033655]  __do_sys_finit_module+0xe9/0x110
[   14.033657]  do_syscall_64+0x33/0x80
[   14.033659]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   14.033660] RIP: 0033:0x7feacf1bef59
[   14.033661] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
[   14.033662] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   14.033663] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
[   14.033663] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
[   14.033664] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
[   14.033664] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
[   14.033665] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
[   14.033666] ---[ end trace 16aeaa08847a13d9 ]---
[   14.033667] ------------[ cut here ]------------
[   14.033668] Bogus possible_crtcs: [ENCODER:73:TMDS-73] possible_crtcs=0xf (full crtc mask=0x7)
[   14.033690] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033690] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
[   14.033704]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
[   14.033706] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E     5.9.0-rc3+ #2
[   14.033707] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
[   14.033719] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
[   14.033721] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
[   14.033721] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
[   14.033722] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
[   14.033723] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
[   14.033723] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
[   14.033724] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf7c000
[   14.033724] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
[   14.033725] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
[   14.033726] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   14.033726] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
[   14.033727] Call Trace:
[   14.033739]  drm_dev_register+0x117/0x1e0 [drm]
[   14.033806]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
[   14.033808]  local_pci_probe+0x42/0x90
[   14.033810]  pci_device_probe+0x108/0x1c0
[   14.033811]  really_probe+0xef/0x4a0
[   14.033813]  driver_probe_device+0xde/0x150
[   14.033814]  device_driver_attach+0x4f/0x60
[   14.033815]  __driver_attach+0x9a/0x140
[   14.033816]  ? device_driver_attach+0x60/0x60
[   14.033817]  bus_for_each_dev+0x76/0xc0
[   14.033818]  ? klist_add_tail+0x3b/0x70
[   14.033820]  bus_add_driver+0x144/0x220
[   14.033821]  ? 0xffffffffc0949000
[   14.033822]  driver_register+0x5b/0xf0
[   14.033823]  ? 0xffffffffc0949000
[   14.033824]  do_one_initcall+0x46/0x1f4
[   14.033825]  ? _cond_resched+0x15/0x40
[   14.033827]  ? kmem_cache_alloc_trace+0x40/0x440
[   14.033828]  ? do_init_module+0x22/0x213
[   14.033829]  do_init_module+0x5b/0x213
[   14.033831]  load_module+0x258c/0x2d30
[   14.033833]  ? __kernel_read+0xf5/0x160
[   14.033834]  ? __do_sys_finit_module+0xe9/0x110
[   14.033835]  __do_sys_finit_module+0xe9/0x110
[   14.033838]  do_syscall_64+0x33/0x80
[   14.033839]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[   14.033840] RIP: 0033:0x7feacf1bef59
[   14.033841] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
[   14.033842] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   14.033843] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
[   14.033843] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
[   14.033844] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
[   14.033844] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
[   14.033845] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
[   14.033846] ---[ end trace 16aeaa08847a13da ]---

Probably the same issue as in
https://bugzilla.kernel.org/show_bug.cgi?id=209123. What does it 
practically mean? Can/should it be silenced in this setup?

Jan
Daniel Vetter Sept. 7, 2020, 7:14 a.m. UTC | #3
On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
>
> On 11.02.20 18:04, Daniel Vetter wrote:
> > On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> WARN if the encoder possible_crtcs is effectively empty or contains
> >> bits for non-existing crtcs.
> >>
> >> v2: Move to drm_mode_config_validate() (Daniel)
> >>     Make the docs say we WARN when this is wrong (Daniel)
> >>     Extract full_crtc_mask()
> >>
> >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > When pushing the fixup needs to be applied before the validation patch
> > here, because we don't want to anger the bisect gods.
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > I think with the fixup we should be good enough with the existing nonsense
> > in drivers. Fingers crossed.
> > -Daniel
> >
> >
> >> ---
> >>  drivers/gpu/drm/drm_mode_config.c | 27 ++++++++++++++++++++++++++-
> >>  include/drm/drm_encoder.h         |  2 +-
> >>  2 files changed, 27 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> >> index afc91447293a..4c1b350ddb95 100644
> >> --- a/drivers/gpu/drm/drm_mode_config.c
> >> +++ b/drivers/gpu/drm/drm_mode_config.c
> >> @@ -581,6 +581,29 @@ static void validate_encoder_possible_clones(struct drm_encoder *encoder)
> >>           encoder->possible_clones, encoder_mask);
> >>  }
> >>
> >> +static u32 full_crtc_mask(struct drm_device *dev)
> >> +{
> >> +    struct drm_crtc *crtc;
> >> +    u32 crtc_mask = 0;
> >> +
> >> +    drm_for_each_crtc(crtc, dev)
> >> +            crtc_mask |= drm_crtc_mask(crtc);
> >> +
> >> +    return crtc_mask;
> >> +}
> >> +
> >> +static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
> >> +{
> >> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
> >> +
> >> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> >> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
> >> +         "Bogus possible_crtcs: "
> >> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> >> +         encoder->base.id, encoder->name,
> >> +         encoder->possible_crtcs, crtc_mask);
> >> +}
> >> +
> >>  void drm_mode_config_validate(struct drm_device *dev)
> >>  {
> >>      struct drm_encoder *encoder;
> >> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct drm_device *dev)
> >>      drm_for_each_encoder(encoder, dev)
> >>              fixup_encoder_possible_clones(encoder);
> >>
> >> -    drm_for_each_encoder(encoder, dev)
> >> +    drm_for_each_encoder(encoder, dev) {
> >>              validate_encoder_possible_clones(encoder);
> >> +            validate_encoder_possible_crtcs(encoder);
> >> +    }
> >>  }
> >> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> >> index 3741963b9587..b236269f41ac 100644
> >> --- a/include/drm/drm_encoder.h
> >> +++ b/include/drm/drm_encoder.h
> >> @@ -142,7 +142,7 @@ struct drm_encoder {
> >>       * the bits for all &drm_crtc objects this encoder can be connected to
> >>       * before calling drm_dev_register().
> >>       *
> >> -     * In reality almost every driver gets this wrong.
> >> +     * You will get a WARN if you get this wrong in the driver.
> >>       *
> >>       * Note that since CRTC objects can't be hotplugged the assigned indices
> >>       * are stable and hence known before registering all objects.
> >> --
> >> 2.24.1
> >>
> >
>
> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):

Adding amdgpu display folks.
-Daniel

>
> [   14.033246] ------------[ cut here ]------------
> [   14.033248] Bogus possible_crtcs: [ENCODER:65:TMDS-65] possible_crtcs=0xf (full crtc mask=0x7)
> [   14.033279] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033279] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> [   14.033306]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> [   14.033310] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G            E     5.9.0-rc3+ #2
> [   14.033311] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
> [   14.033324] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033327] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> [   14.033328] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> [   14.033329] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
> [   14.033330] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
> [   14.033331] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
> [   14.033331] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf8b800
> [   14.033332] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
> [   14.033333] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
> [   14.033334] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   14.033335] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
> [   14.033335] Call Trace:
> [   14.033350]  drm_dev_register+0x117/0x1e0 [drm]
> [   14.033423]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> [   14.033428]  local_pci_probe+0x42/0x90
> [   14.033430]  pci_device_probe+0x108/0x1c0
> [   14.033433]  really_probe+0xef/0x4a0
> [   14.033435]  driver_probe_device+0xde/0x150
> [   14.033436]  device_driver_attach+0x4f/0x60
> [   14.033438]  __driver_attach+0x9a/0x140
> [   14.033439]  ? device_driver_attach+0x60/0x60
> [   14.033441]  bus_for_each_dev+0x76/0xc0
> [   14.033443]  ? klist_add_tail+0x3b/0x70
> [   14.033445]  bus_add_driver+0x144/0x220
> [   14.033446]  ? 0xffffffffc0949000
> [   14.033447]  driver_register+0x5b/0xf0
> [   14.033448]  ? 0xffffffffc0949000
> [   14.033451]  do_one_initcall+0x46/0x1f4
> [   14.033454]  ? _cond_resched+0x15/0x40
> [   14.033456]  ? kmem_cache_alloc_trace+0x40/0x440
> [   14.033459]  ? do_init_module+0x22/0x213
> [   14.033460]  do_init_module+0x5b/0x213
> [   14.033462]  load_module+0x258c/0x2d30
> [   14.033465]  ? __kernel_read+0xf5/0x160
> [   14.033467]  ? __do_sys_finit_module+0xe9/0x110
> [   14.033468]  __do_sys_finit_module+0xe9/0x110
> [   14.033471]  do_syscall_64+0x33/0x80
> [   14.033473]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [   14.033474] RIP: 0033:0x7feacf1bef59
> [   14.033477] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> [   14.033477] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> [   14.033478] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
> [   14.033479] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
> [   14.033479] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
> [   14.033480] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
> [   14.033480] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
> [   14.033482] ---[ end trace 16aeaa08847a13d8 ]---
> [   14.033483] ------------[ cut here ]------------
> [   14.033484] Bogus possible_crtcs: [ENCODER:69:TMDS-69] possible_crtcs=0xf (full crtc mask=0x7)
> [   14.033507] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033507] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> [   14.033522]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> [   14.033524] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E     5.9.0-rc3+ #2
> [   14.033525] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
> [   14.033538] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033539] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> [   14.033540] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> [   14.033540] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
> [   14.033541] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
> [   14.033542] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
> [   14.033542] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf89800
> [   14.033543] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
> [   14.033544] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
> [   14.033544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   14.033545] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
> [   14.033545] Call Trace:
> [   14.033557]  drm_dev_register+0x117/0x1e0 [drm]
> [   14.033625]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> [   14.033627]  local_pci_probe+0x42/0x90
> [   14.033629]  pci_device_probe+0x108/0x1c0
> [   14.033630]  really_probe+0xef/0x4a0
> [   14.033632]  driver_probe_device+0xde/0x150
> [   14.033633]  device_driver_attach+0x4f/0x60
> [   14.033634]  __driver_attach+0x9a/0x140
> [   14.033635]  ? device_driver_attach+0x60/0x60
> [   14.033636]  bus_for_each_dev+0x76/0xc0
> [   14.033638]  ? klist_add_tail+0x3b/0x70
> [   14.033639]  bus_add_driver+0x144/0x220
> [   14.033640]  ? 0xffffffffc0949000
> [   14.033641]  driver_register+0x5b/0xf0
> [   14.033642]  ? 0xffffffffc0949000
> [   14.033643]  do_one_initcall+0x46/0x1f4
> [   14.033645]  ? _cond_resched+0x15/0x40
> [   14.033646]  ? kmem_cache_alloc_trace+0x40/0x440
> [   14.033648]  ? do_init_module+0x22/0x213
> [   14.033649]  do_init_module+0x5b/0x213
> [   14.033650]  load_module+0x258c/0x2d30
> [   14.033652]  ? __kernel_read+0xf5/0x160
> [   14.033654]  ? __do_sys_finit_module+0xe9/0x110
> [   14.033655]  __do_sys_finit_module+0xe9/0x110
> [   14.033657]  do_syscall_64+0x33/0x80
> [   14.033659]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [   14.033660] RIP: 0033:0x7feacf1bef59
> [   14.033661] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> [   14.033662] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> [   14.033663] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
> [   14.033663] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
> [   14.033664] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
> [   14.033664] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
> [   14.033665] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
> [   14.033666] ---[ end trace 16aeaa08847a13d9 ]---
> [   14.033667] ------------[ cut here ]------------
> [   14.033668] Bogus possible_crtcs: [ENCODER:73:TMDS-73] possible_crtcs=0xf (full crtc mask=0x7)
> [   14.033690] WARNING: CPU: 0 PID: 282 at ../drivers/gpu/drm/drm_mode_config.c:622 drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033690] Modules linked in: amdgpu(E+) mfd_core(E) snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E) snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E) drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E) snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E) snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E) snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E) glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E) sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E) acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E) crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E) r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E) i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> [   14.033704]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> [   14.033706] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E     5.9.0-rc3+ #2
> [   14.033707] Hardware name: Default string Default string/Default string, BIOS 5.0.1.4 02/14/2020
> [   14.033719] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> [   14.033721] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> [   14.033721] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> [   14.033722] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX: 0000000000000027
> [   14.033723] RDX: 0000000000000000 RSI: 0000000000000086 RDI: ffff9c6910a18ac8
> [   14.033723] RBP: 0000000000000001 R08: 0000000000000064 R09: ffffffff9784a724
> [   14.033724] R10: 0000000000000001 R11: 0000000000000000 R12: ffff9c690bf7c000
> [   14.033724] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15: 000000000000003f
> [   14.033725] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000) knlGS:0000000000000000
> [   14.033726] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   14.033726] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4: 00000000003506f0
> [   14.033727] Call Trace:
> [   14.033739]  drm_dev_register+0x117/0x1e0 [drm]
> [   14.033806]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> [   14.033808]  local_pci_probe+0x42/0x90
> [   14.033810]  pci_device_probe+0x108/0x1c0
> [   14.033811]  really_probe+0xef/0x4a0
> [   14.033813]  driver_probe_device+0xde/0x150
> [   14.033814]  device_driver_attach+0x4f/0x60
> [   14.033815]  __driver_attach+0x9a/0x140
> [   14.033816]  ? device_driver_attach+0x60/0x60
> [   14.033817]  bus_for_each_dev+0x76/0xc0
> [   14.033818]  ? klist_add_tail+0x3b/0x70
> [   14.033820]  bus_add_driver+0x144/0x220
> [   14.033821]  ? 0xffffffffc0949000
> [   14.033822]  driver_register+0x5b/0xf0
> [   14.033823]  ? 0xffffffffc0949000
> [   14.033824]  do_one_initcall+0x46/0x1f4
> [   14.033825]  ? _cond_resched+0x15/0x40
> [   14.033827]  ? kmem_cache_alloc_trace+0x40/0x440
> [   14.033828]  ? do_init_module+0x22/0x213
> [   14.033829]  do_init_module+0x5b/0x213
> [   14.033831]  load_module+0x258c/0x2d30
> [   14.033833]  ? __kernel_read+0xf5/0x160
> [   14.033834]  ? __do_sys_finit_module+0xe9/0x110
> [   14.033835]  __do_sys_finit_module+0xe9/0x110
> [   14.033838]  do_syscall_64+0x33/0x80
> [   14.033839]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [   14.033840] RIP: 0033:0x7feacf1bef59
> [   14.033841] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> [   14.033842] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> [   14.033843] RAX: ffffffffffffffda RBX: 0000565489749410 RCX: 00007feacf1bef59
> [   14.033843] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI: 0000000000000015
> [   14.033844] RBP: 00007feacf0c3cad R08: 0000000000000000 R09: 0000000000000000
> [   14.033844] R10: 0000000000000015 R11: 0000000000000246 R12: 0000000000000000
> [   14.033845] R13: 00005654897472c0 R14: 0000000000020000 R15: 0000565489749410
> [   14.033846] ---[ end trace 16aeaa08847a13da ]---
>
> Probably the same issue as in
> https://bugzilla.kernel.org/show_bug.cgi?id=209123. What does it
> practically mean? Can/should it be silenced in this setup?
>
> Jan
Alex Deucher Sept. 10, 2020, 6:18 p.m. UTC | #4
[AMD Public Use]



> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Daniel Vetter
> Sent: Monday, September 7, 2020 3:15 AM
> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
> gfx@lists.freedesktop.org>; Thomas Zimmermann
> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
> 
> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
> >
> > On 11.02.20 18:04, Daniel Vetter wrote:
> > > On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> > >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>
> > >> WARN if the encoder possible_crtcs is effectively empty or contains
> > >> bits for non-existing crtcs.
> > >>
> > >> v2: Move to drm_mode_config_validate() (Daniel)
> > >>     Make the docs say we WARN when this is wrong (Daniel)
> > >>     Extract full_crtc_mask()
> > >>
> > >> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > >> Cc: Daniel Vetter <daniel@ffwll.ch>
> > >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > When pushing the fixup needs to be applied before the validation
> > > patch here, because we don't want to anger the bisect gods.
> > >
> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >
> > > I think with the fixup we should be good enough with the existing
> > > nonsense in drivers. Fingers crossed.
> > > -Daniel
> > >
> > >
> > >> ---
> > >>  drivers/gpu/drm/drm_mode_config.c | 27
> ++++++++++++++++++++++++++-
> > >>  include/drm/drm_encoder.h         |  2 +-
> > >>  2 files changed, 27 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_mode_config.c
> > >> b/drivers/gpu/drm/drm_mode_config.c
> > >> index afc91447293a..4c1b350ddb95 100644
> > >> --- a/drivers/gpu/drm/drm_mode_config.c
> > >> +++ b/drivers/gpu/drm/drm_mode_config.c
> > >> @@ -581,6 +581,29 @@ static void
> validate_encoder_possible_clones(struct drm_encoder *encoder)
> > >>           encoder->possible_clones, encoder_mask);  }
> > >>
> > >> +static u32 full_crtc_mask(struct drm_device *dev) {
> > >> +    struct drm_crtc *crtc;
> > >> +    u32 crtc_mask = 0;
> > >> +
> > >> +    drm_for_each_crtc(crtc, dev)
> > >> +            crtc_mask |= drm_crtc_mask(crtc);
> > >> +
> > >> +    return crtc_mask;
> > >> +}
> > >> +
> > >> +static void validate_encoder_possible_crtcs(struct drm_encoder
> > >> +*encoder) {
> > >> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
> > >> +
> > >> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> > >> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
> > >> +         "Bogus possible_crtcs: "
> > >> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> > >> +         encoder->base.id, encoder->name,
> > >> +         encoder->possible_crtcs, crtc_mask); }
> > >> +
> > >>  void drm_mode_config_validate(struct drm_device *dev)  {
> > >>      struct drm_encoder *encoder;
> > >> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
> drm_device *dev)
> > >>      drm_for_each_encoder(encoder, dev)
> > >>              fixup_encoder_possible_clones(encoder);
> > >>
> > >> -    drm_for_each_encoder(encoder, dev)
> > >> +    drm_for_each_encoder(encoder, dev) {
> > >>              validate_encoder_possible_clones(encoder);
> > >> +            validate_encoder_possible_crtcs(encoder);
> > >> +    }
> > >>  }
> > >> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > >> index 3741963b9587..b236269f41ac 100644
> > >> --- a/include/drm/drm_encoder.h
> > >> +++ b/include/drm/drm_encoder.h
> > >> @@ -142,7 +142,7 @@ struct drm_encoder {
> > >>       * the bits for all &drm_crtc objects this encoder can be connected to
> > >>       * before calling drm_dev_register().
> > >>       *
> > >> -     * In reality almost every driver gets this wrong.
> > >> +     * You will get a WARN if you get this wrong in the driver.
> > >>       *
> > >>       * Note that since CRTC objects can't be hotplugged the assigned
> indices
> > >>       * are stable and hence known before registering all objects.
> > >> --
> > >> 2.24.1
> > >>
> > >
> >
> > Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
> 
> Adding amdgpu display folks.

I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.

Alex


> -Daniel
> 
> >
> > [   14.033246] ------------[ cut here ]------------
> > [   14.033248] Bogus possible_crtcs: [ENCODER:65:TMDS-65]
> possible_crtcs=0xf (full crtc mask=0x7)
> > [   14.033279] WARNING: CPU: 0 PID: 282 at
> ../drivers/gpu/drm/drm_mode_config.c:622
> drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033279] Modules linked in: amdgpu(E+) mfd_core(E)
> snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E)
> snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E)
> drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E)
> snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E)
> snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E)
> snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E)
> glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E)
> sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E)
> acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E)
> efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E)
> crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E)
> crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E)
> r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E)
> i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> > [   14.033306]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> > [   14.033310] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G            E
> 5.9.0-rc3+ #2
> > [   14.033311] Hardware name: Default string Default string/Default string,
> BIOS 5.0.1.4 02/14/2020
> > [   14.033324] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033327] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b
> 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b
> 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> > [   14.033328] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> > [   14.033329] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX:
> 0000000000000027
> > [   14.033330] RDX: 0000000000000000 RSI: 0000000000000086 RDI:
> ffff9c6910a18ac8
> > [   14.033331] RBP: 0000000000000001 R08: 0000000000000064 R09:
> ffffffff9784a724
> > [   14.033331] R10: 0000000000000001 R11: 0000000000000000 R12:
> ffff9c690bf8b800
> > [   14.033332] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15:
> 000000000000003f
> > [   14.033333] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000)
> knlGS:0000000000000000
> > [   14.033334] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   14.033335] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4:
> 00000000003506f0
> > [   14.033335] Call Trace:
> > [   14.033350]  drm_dev_register+0x117/0x1e0 [drm]
> > [   14.033423]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> > [   14.033428]  local_pci_probe+0x42/0x90
> > [   14.033430]  pci_device_probe+0x108/0x1c0
> > [   14.033433]  really_probe+0xef/0x4a0
> > [   14.033435]  driver_probe_device+0xde/0x150
> > [   14.033436]  device_driver_attach+0x4f/0x60
> > [   14.033438]  __driver_attach+0x9a/0x140
> > [   14.033439]  ? device_driver_attach+0x60/0x60
> > [   14.033441]  bus_for_each_dev+0x76/0xc0
> > [   14.033443]  ? klist_add_tail+0x3b/0x70
> > [   14.033445]  bus_add_driver+0x144/0x220
> > [   14.033446]  ? 0xffffffffc0949000
> > [   14.033447]  driver_register+0x5b/0xf0
> > [   14.033448]  ? 0xffffffffc0949000
> > [   14.033451]  do_one_initcall+0x46/0x1f4
> > [   14.033454]  ? _cond_resched+0x15/0x40
> > [   14.033456]  ? kmem_cache_alloc_trace+0x40/0x440
> > [   14.033459]  ? do_init_module+0x22/0x213
> > [   14.033460]  do_init_module+0x5b/0x213
> > [   14.033462]  load_module+0x258c/0x2d30
> > [   14.033465]  ? __kernel_read+0xf5/0x160
> > [   14.033467]  ? __do_sys_finit_module+0xe9/0x110
> > [   14.033468]  __do_sys_finit_module+0xe9/0x110
> > [   14.033471]  do_syscall_64+0x33/0x80
> > [   14.033473]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [   14.033474] RIP: 0033:0x7feacf1bef59
> > [   14.033477] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8
> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> > [   14.033477] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> > [   14.033478] RAX: ffffffffffffffda RBX: 0000565489749410 RCX:
> 00007feacf1bef59
> > [   14.033479] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI:
> 0000000000000015
> > [   14.033479] RBP: 00007feacf0c3cad R08: 0000000000000000 R09:
> 0000000000000000
> > [   14.033480] R10: 0000000000000015 R11: 0000000000000246 R12:
> 0000000000000000
> > [   14.033480] R13: 00005654897472c0 R14: 0000000000020000 R15:
> 0000565489749410
> > [   14.033482] ---[ end trace 16aeaa08847a13d8 ]---
> > [   14.033483] ------------[ cut here ]------------
> > [   14.033484] Bogus possible_crtcs: [ENCODER:69:TMDS-69]
> possible_crtcs=0xf (full crtc mask=0x7)
> > [   14.033507] WARNING: CPU: 0 PID: 282 at
> ../drivers/gpu/drm/drm_mode_config.c:622
> drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033507] Modules linked in: amdgpu(E+) mfd_core(E)
> snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E)
> snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E)
> drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E)
> snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E)
> snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E)
> snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E)
> glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E)
> sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E)
> acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E)
> efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E)
> crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E)
> crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E)
> r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E)
> i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> > [   14.033522]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> > [   14.033524] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E
> 5.9.0-rc3+ #2
> > [   14.033525] Hardware name: Default string Default string/Default string,
> BIOS 5.0.1.4 02/14/2020
> > [   14.033538] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033539] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b
> 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b
> 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> > [   14.033540] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> > [   14.033540] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX:
> 0000000000000027
> > [   14.033541] RDX: 0000000000000000 RSI: 0000000000000086 RDI:
> ffff9c6910a18ac8
> > [   14.033542] RBP: 0000000000000001 R08: 0000000000000064 R09:
> ffffffff9784a724
> > [   14.033542] R10: 0000000000000001 R11: 0000000000000000 R12:
> ffff9c690bf89800
> > [   14.033543] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15:
> 000000000000003f
> > [   14.033544] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000)
> knlGS:0000000000000000
> > [   14.033544] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   14.033545] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4:
> 00000000003506f0
> > [   14.033545] Call Trace:
> > [   14.033557]  drm_dev_register+0x117/0x1e0 [drm]
> > [   14.033625]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> > [   14.033627]  local_pci_probe+0x42/0x90
> > [   14.033629]  pci_device_probe+0x108/0x1c0
> > [   14.033630]  really_probe+0xef/0x4a0
> > [   14.033632]  driver_probe_device+0xde/0x150
> > [   14.033633]  device_driver_attach+0x4f/0x60
> > [   14.033634]  __driver_attach+0x9a/0x140
> > [   14.033635]  ? device_driver_attach+0x60/0x60
> > [   14.033636]  bus_for_each_dev+0x76/0xc0
> > [   14.033638]  ? klist_add_tail+0x3b/0x70
> > [   14.033639]  bus_add_driver+0x144/0x220
> > [   14.033640]  ? 0xffffffffc0949000
> > [   14.033641]  driver_register+0x5b/0xf0
> > [   14.033642]  ? 0xffffffffc0949000
> > [   14.033643]  do_one_initcall+0x46/0x1f4
> > [   14.033645]  ? _cond_resched+0x15/0x40
> > [   14.033646]  ? kmem_cache_alloc_trace+0x40/0x440
> > [   14.033648]  ? do_init_module+0x22/0x213
> > [   14.033649]  do_init_module+0x5b/0x213
> > [   14.033650]  load_module+0x258c/0x2d30
> > [   14.033652]  ? __kernel_read+0xf5/0x160
> > [   14.033654]  ? __do_sys_finit_module+0xe9/0x110
> > [   14.033655]  __do_sys_finit_module+0xe9/0x110
> > [   14.033657]  do_syscall_64+0x33/0x80
> > [   14.033659]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [   14.033660] RIP: 0033:0x7feacf1bef59
> > [   14.033661] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8
> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> > [   14.033662] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> > [   14.033663] RAX: ffffffffffffffda RBX: 0000565489749410 RCX:
> 00007feacf1bef59
> > [   14.033663] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI:
> 0000000000000015
> > [   14.033664] RBP: 00007feacf0c3cad R08: 0000000000000000 R09:
> 0000000000000000
> > [   14.033664] R10: 0000000000000015 R11: 0000000000000246 R12:
> 0000000000000000
> > [   14.033665] R13: 00005654897472c0 R14: 0000000000020000 R15:
> 0000565489749410
> > [   14.033666] ---[ end trace 16aeaa08847a13d9 ]---
> > [   14.033667] ------------[ cut here ]------------
> > [   14.033668] Bogus possible_crtcs: [ENCODER:73:TMDS-73]
> possible_crtcs=0xf (full crtc mask=0x7)
> > [   14.033690] WARNING: CPU: 0 PID: 282 at
> ../drivers/gpu/drm/drm_mode_config.c:622
> drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033690] Modules linked in: amdgpu(E+) mfd_core(E)
> snd_hda_codec_realtek(E) kvm_amd(E) gpu_sched(E) i2c_algo_bit(E) ttm(E)
> snd_hda_codec_generic(E) kvm(E) ledtrig_audio(E) snd_hda_codec_hdmi(E)
> drm_kms_helper(E) snd_hda_intel(E) snd_intel_dspcfg(E)
> snd_hda_codec(E) cec(E) snd_hwdep(E) drm(E) irqbypass(E)
> snd_hda_core(E) crc32_pclmul(E) snd_pcm(E) ghash_clmulni_intel(E)
> snd_timer(E) sg(E) aesni_intel(E) snd(E) ccp(E) soundcore(E) rng_core(E)
> glue_helper(E) libaes(E) crypto_simd(E) k10temp(E) efi_pstore(E)
> sp5100_tco(E) evdev(E) cryptd(E) pcspkr(E) efivars(E) video(E) button(E)
> acpi_cpufreq(E) w83627hf_wdt(E) watchdog(E) nct6775(E) hwmon_vid(E)
> efivarfs(E) ip_tables(E) x_tables(E) autofs4(E) ext4(E) crc32c_generic(E)
> crc16(E) mbcache(E) jbd2(E) sd_mod(E) t10_pi(E) crc_t10dif(E)
> crct10dif_generic(E) uas(E) usb_storage(E) ahci(E) libahci(E) xhci_pci(E)
> r8169(E) realtek(E) mdio_devres(E) xhci_hcd(E) libata(E)
> i2c_amd_mp2_pci(E) crct10dif_pclmul(E) crct10dif_common(E) scsi_mod(E)
> > [   14.033704]  crc32c_intel(E) i2c_piix4(E) usbcore(E) libphy(E)
> > [   14.033706] CPU: 0 PID: 282 Comm: systemd-udevd Tainted: G        W   E
> 5.9.0-rc3+ #2
> > [   14.033707] Hardware name: Default string Default string/Default string,
> BIOS 5.0.1.4 02/14/2020
> > [   14.033719] RIP: 0010:drm_mode_config_validate+0x17d/0x200 [drm]
> > [   14.033721] Code: 42 f0 75 e6 41 85 f8 74 09 44 89 c0 f7 d0 85 f8 74 1a 49 8b
> 54 24 38 41 8b 74 24 18 89 f9 48 c7 c7 80 7d 70 c0 e8 13 66 9a d5 <0f> 0b 49 8b
> 44 24 08 49 39 c5 4c 8d 60 f8 0f 85 e9 fe ff ff 5b 5d
> > [   14.033721] RSP: 0018:ffffae5f404b7a90 EFLAGS: 00010282
> > [   14.033722] RAX: 0000000000000000 RBX: ffff9c6907bd0ad0 RCX:
> 0000000000000027
> > [   14.033723] RDX: 0000000000000000 RSI: 0000000000000086 RDI:
> ffff9c6910a18ac8
> > [   14.033723] RBP: 0000000000000001 R08: 0000000000000064 R09:
> ffffffff9784a724
> > [   14.033724] R10: 0000000000000001 R11: 0000000000000000 R12:
> ffff9c690bf7c000
> > [   14.033724] R13: ffff9c6907bd0ad8 R14: ffff9c6907bd0ad8 R15:
> 000000000000003f
> > [   14.033725] FS:  00007feace9d4d40(0000) GS:ffff9c6910a00000(0000)
> knlGS:0000000000000000
> > [   14.033726] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   14.033726] CR2: 0000557a37b7e270 CR3: 000000038d5fa000 CR4:
> 00000000003506f0
> > [   14.033727] Call Trace:
> > [   14.033739]  drm_dev_register+0x117/0x1e0 [drm]
> > [   14.033806]  amdgpu_pci_probe+0x134/0x200 [amdgpu]
> > [   14.033808]  local_pci_probe+0x42/0x90
> > [   14.033810]  pci_device_probe+0x108/0x1c0
> > [   14.033811]  really_probe+0xef/0x4a0
> > [   14.033813]  driver_probe_device+0xde/0x150
> > [   14.033814]  device_driver_attach+0x4f/0x60
> > [   14.033815]  __driver_attach+0x9a/0x140
> > [   14.033816]  ? device_driver_attach+0x60/0x60
> > [   14.033817]  bus_for_each_dev+0x76/0xc0
> > [   14.033818]  ? klist_add_tail+0x3b/0x70
> > [   14.033820]  bus_add_driver+0x144/0x220
> > [   14.033821]  ? 0xffffffffc0949000
> > [   14.033822]  driver_register+0x5b/0xf0
> > [   14.033823]  ? 0xffffffffc0949000
> > [   14.033824]  do_one_initcall+0x46/0x1f4
> > [   14.033825]  ? _cond_resched+0x15/0x40
> > [   14.033827]  ? kmem_cache_alloc_trace+0x40/0x440
> > [   14.033828]  ? do_init_module+0x22/0x213
> > [   14.033829]  do_init_module+0x5b/0x213
> > [   14.033831]  load_module+0x258c/0x2d30
> > [   14.033833]  ? __kernel_read+0xf5/0x160
> > [   14.033834]  ? __do_sys_finit_module+0xe9/0x110
> > [   14.033835]  __do_sys_finit_module+0xe9/0x110
> > [   14.033838]  do_syscall_64+0x33/0x80
> > [   14.033839]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [   14.033840] RIP: 0033:0x7feacf1bef59
> > [   14.033841] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8
> 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 73 01 c3 48 8b 0d 07 6f 0c 00 f7 d8 64 89 01 48
> > [   14.033842] RSP: 002b:00007ffd03534438 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000139
> > [   14.033843] RAX: ffffffffffffffda RBX: 0000565489749410 RCX:
> 00007feacf1bef59
> > [   14.033843] RDX: 0000000000000000 RSI: 00007feacf0c3cad RDI:
> 0000000000000015
> > [   14.033844] RBP: 00007feacf0c3cad R08: 0000000000000000 R09:
> 0000000000000000
> > [   14.033844] R10: 0000000000000015 R11: 0000000000000246 R12:
> 0000000000000000
> > [   14.033845] R13: 00005654897472c0 R14: 0000000000020000 R15:
> 0000565489749410
> > [   14.033846] ---[ end trace 16aeaa08847a13da ]---
> >
> > Probably the same issue as in
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> >
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D209123&amp;data=02%7C01%7Cal
> exander.deucher%40amd.com%7C28e5e2e1aecd417e7dd608d852fdc391%7C
> 3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637350597170905726&a
> mp;sdata=z9smK6qFP6akCsGs4ToJNigPHke7SUfnb3i2F72u7S0%3D&amp;res
> erved=0. What does it practically mean? Can/should it be silenced in this
> setup?
> >
> > Jan
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.f
> fwll.ch%2F&amp;data=02%7C01%7Calexander.deucher%40amd.com%7C28e
> 5e2e1aecd417e7dd608d852fdc391%7C3dd8961fe4884e608e11a82d994e183d
> %7C0%7C0%7C637350597170905726&amp;sdata=6fPvyO%2FFxcf7mu4j4l%2B
> OlwMe3BsaKY3zulTE3F%2BfeIw%3D&amp;reserved=0
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> freedesktop.org%2Fmailman%2Flistinfo%2Famd-
> gfx&amp;data=02%7C01%7Calexander.deucher%40amd.com%7C28e5e2e1a
> ecd417e7dd608d852fdc391%7C3dd8961fe4884e608e11a82d994e183d%7C0%
> 7C0%7C637350597170905726&amp;sdata=%2FV0U72P9e8Vu8NWON7lJdx7k3
> TtMm9TjBz3D%2B7NtvOo%3D&amp;reserved=0
Jan Kiszka Sept. 29, 2020, 9:36 a.m. UTC | #5
On 10.09.20 20:18, Deucher, Alexander wrote:
> [AMD Public Use]
>
>
>
>> -----Original Message-----
>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Daniel Vetter
>> Sent: Monday, September 7, 2020 3:15 AM
>> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
>> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
>> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
>> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
>> gfx@lists.freedesktop.org>; Thomas Zimmermann
>> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
>> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
>>
>> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
>>>
>>> On 11.02.20 18:04, Daniel Vetter wrote:
>>>> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> WARN if the encoder possible_crtcs is effectively empty or contains
>>>>> bits for non-existing crtcs.
>>>>>
>>>>> v2: Move to drm_mode_config_validate() (Daniel)
>>>>>     Make the docs say we WARN when this is wrong (Daniel)
>>>>>     Extract full_crtc_mask()
>>>>>
>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> When pushing the fixup needs to be applied before the validation
>>>> patch here, because we don't want to anger the bisect gods.
>>>>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>
>>>> I think with the fixup we should be good enough with the existing
>>>> nonsense in drivers. Fingers crossed.
>>>> -Daniel
>>>>
>>>>
>>>>> ---
>>>>>  drivers/gpu/drm/drm_mode_config.c | 27
>> ++++++++++++++++++++++++++-
>>>>>  include/drm/drm_encoder.h         |  2 +-
>>>>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>>>>> b/drivers/gpu/drm/drm_mode_config.c
>>>>> index afc91447293a..4c1b350ddb95 100644
>>>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>>>> @@ -581,6 +581,29 @@ static void
>> validate_encoder_possible_clones(struct drm_encoder *encoder)
>>>>>           encoder->possible_clones, encoder_mask);  }
>>>>>
>>>>> +static u32 full_crtc_mask(struct drm_device *dev) {
>>>>> +    struct drm_crtc *crtc;
>>>>> +    u32 crtc_mask = 0;
>>>>> +
>>>>> +    drm_for_each_crtc(crtc, dev)
>>>>> +            crtc_mask |= drm_crtc_mask(crtc);
>>>>> +
>>>>> +    return crtc_mask;
>>>>> +}
>>>>> +
>>>>> +static void validate_encoder_possible_crtcs(struct drm_encoder
>>>>> +*encoder) {
>>>>> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
>>>>> +
>>>>> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
>>>>> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
>>>>> +         "Bogus possible_crtcs: "
>>>>> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
>>>>> +         encoder->base.id, encoder->name,
>>>>> +         encoder->possible_crtcs, crtc_mask); }
>>>>> +
>>>>>  void drm_mode_config_validate(struct drm_device *dev)  {
>>>>>      struct drm_encoder *encoder;
>>>>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
>> drm_device *dev)
>>>>>      drm_for_each_encoder(encoder, dev)
>>>>>              fixup_encoder_possible_clones(encoder);
>>>>>
>>>>> -    drm_for_each_encoder(encoder, dev)
>>>>> +    drm_for_each_encoder(encoder, dev) {
>>>>>              validate_encoder_possible_clones(encoder);
>>>>> +            validate_encoder_possible_crtcs(encoder);
>>>>> +    }
>>>>>  }
>>>>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
>>>>> index 3741963b9587..b236269f41ac 100644
>>>>> --- a/include/drm/drm_encoder.h
>>>>> +++ b/include/drm/drm_encoder.h
>>>>> @@ -142,7 +142,7 @@ struct drm_encoder {
>>>>>       * the bits for all &drm_crtc objects this encoder can be connected to
>>>>>       * before calling drm_dev_register().
>>>>>       *
>>>>> -     * In reality almost every driver gets this wrong.
>>>>> +     * You will get a WARN if you get this wrong in the driver.
>>>>>       *
>>>>>       * Note that since CRTC objects can't be hotplugged the assigned
>> indices
>>>>>       * are stable and hence known before registering all objects.
>>>>> --
>>>>> 2.24.1
>>>>>
>>>>
>>>
>>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
>>
>> Adding amdgpu display folks.
>
> I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.
>

So, will this be fixed any time soon? I don't feel qualified writing
such a patch but I would obviously be happy to test one.

Jan
Alex Deucher Sept. 29, 2020, 8:04 p.m. UTC | #6
On Tue, Sep 29, 2020 at 8:31 AM Jan Kiszka <jan.kiszka@web.de> wrote:
>
> On 10.09.20 20:18, Deucher, Alexander wrote:
> > [AMD Public Use]
> >
> >
> >
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> >> Daniel Vetter
> >> Sent: Monday, September 7, 2020 3:15 AM
> >> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
> >> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
> >> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
> >> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
> >> gfx@lists.freedesktop.org>; Thomas Zimmermann
> >> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
> >> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
> >>
> >> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
> >>>
> >>> On 11.02.20 18:04, Daniel Vetter wrote:
> >>>> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>
> >>>>> WARN if the encoder possible_crtcs is effectively empty or contains
> >>>>> bits for non-existing crtcs.
> >>>>>
> >>>>> v2: Move to drm_mode_config_validate() (Daniel)
> >>>>>     Make the docs say we WARN when this is wrong (Daniel)
> >>>>>     Extract full_crtc_mask()
> >>>>>
> >>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> >>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>
> >>>> When pushing the fixup needs to be applied before the validation
> >>>> patch here, because we don't want to anger the bisect gods.
> >>>>
> >>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>>>
> >>>> I think with the fixup we should be good enough with the existing
> >>>> nonsense in drivers. Fingers crossed.
> >>>> -Daniel
> >>>>
> >>>>
> >>>>> ---
> >>>>>  drivers/gpu/drm/drm_mode_config.c | 27
> >> ++++++++++++++++++++++++++-
> >>>>>  include/drm/drm_encoder.h         |  2 +-
> >>>>>  2 files changed, 27 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
> >>>>> b/drivers/gpu/drm/drm_mode_config.c
> >>>>> index afc91447293a..4c1b350ddb95 100644
> >>>>> --- a/drivers/gpu/drm/drm_mode_config.c
> >>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
> >>>>> @@ -581,6 +581,29 @@ static void
> >> validate_encoder_possible_clones(struct drm_encoder *encoder)
> >>>>>           encoder->possible_clones, encoder_mask);  }
> >>>>>
> >>>>> +static u32 full_crtc_mask(struct drm_device *dev) {
> >>>>> +    struct drm_crtc *crtc;
> >>>>> +    u32 crtc_mask = 0;
> >>>>> +
> >>>>> +    drm_for_each_crtc(crtc, dev)
> >>>>> +            crtc_mask |= drm_crtc_mask(crtc);
> >>>>> +
> >>>>> +    return crtc_mask;
> >>>>> +}
> >>>>> +
> >>>>> +static void validate_encoder_possible_crtcs(struct drm_encoder
> >>>>> +*encoder) {
> >>>>> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
> >>>>> +
> >>>>> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> >>>>> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
> >>>>> +         "Bogus possible_crtcs: "
> >>>>> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> >>>>> +         encoder->base.id, encoder->name,
> >>>>> +         encoder->possible_crtcs, crtc_mask); }
> >>>>> +
> >>>>>  void drm_mode_config_validate(struct drm_device *dev)  {
> >>>>>      struct drm_encoder *encoder;
> >>>>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
> >> drm_device *dev)
> >>>>>      drm_for_each_encoder(encoder, dev)
> >>>>>              fixup_encoder_possible_clones(encoder);
> >>>>>
> >>>>> -    drm_for_each_encoder(encoder, dev)
> >>>>> +    drm_for_each_encoder(encoder, dev) {
> >>>>>              validate_encoder_possible_clones(encoder);
> >>>>> +            validate_encoder_possible_crtcs(encoder);
> >>>>> +    }
> >>>>>  }
> >>>>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> >>>>> index 3741963b9587..b236269f41ac 100644
> >>>>> --- a/include/drm/drm_encoder.h
> >>>>> +++ b/include/drm/drm_encoder.h
> >>>>> @@ -142,7 +142,7 @@ struct drm_encoder {
> >>>>>       * the bits for all &drm_crtc objects this encoder can be connected to
> >>>>>       * before calling drm_dev_register().
> >>>>>       *
> >>>>> -     * In reality almost every driver gets this wrong.
> >>>>> +     * You will get a WARN if you get this wrong in the driver.
> >>>>>       *
> >>>>>       * Note that since CRTC objects can't be hotplugged the assigned
> >> indices
> >>>>>       * are stable and hence known before registering all objects.
> >>>>> --
> >>>>> 2.24.1
> >>>>>
> >>>>
> >>>
> >>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
> >>
> >> Adding amdgpu display folks.
> >
> > I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.
> >
>
> So, will this be fixed any time soon? I don't feel qualified writing
> such a patch but I would obviously be happy to test one.

It's harmless, but I'll send out a patch soon.

Alex
Alex Deucher Dec. 3, 2020, 9:30 p.m. UTC | #7
On Tue, Sep 29, 2020 at 4:04 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Sep 29, 2020 at 8:31 AM Jan Kiszka <jan.kiszka@web.de> wrote:
> >
> > On 10.09.20 20:18, Deucher, Alexander wrote:
> > > [AMD Public Use]
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > >> Daniel Vetter
> > >> Sent: Monday, September 7, 2020 3:15 AM
> > >> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
> > >> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
> > >> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
> > >> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
> > >> gfx@lists.freedesktop.org>; Thomas Zimmermann
> > >> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
> > >> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
> > >>
> > >> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
> > >>>
> > >>> On 11.02.20 18:04, Daniel Vetter wrote:
> > >>>> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> > >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>>>>
> > >>>>> WARN if the encoder possible_crtcs is effectively empty or contains
> > >>>>> bits for non-existing crtcs.
> > >>>>>
> > >>>>> v2: Move to drm_mode_config_validate() (Daniel)
> > >>>>>     Make the docs say we WARN when this is wrong (Daniel)
> > >>>>>     Extract full_crtc_mask()
> > >>>>>
> > >>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > >>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> > >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>>>
> > >>>> When pushing the fixup needs to be applied before the validation
> > >>>> patch here, because we don't want to anger the bisect gods.
> > >>>>
> > >>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > >>>>
> > >>>> I think with the fixup we should be good enough with the existing
> > >>>> nonsense in drivers. Fingers crossed.
> > >>>> -Daniel
> > >>>>
> > >>>>
> > >>>>> ---
> > >>>>>  drivers/gpu/drm/drm_mode_config.c | 27
> > >> ++++++++++++++++++++++++++-
> > >>>>>  include/drm/drm_encoder.h         |  2 +-
> > >>>>>  2 files changed, 27 insertions(+), 2 deletions(-)
> > >>>>>
> > >>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
> > >>>>> b/drivers/gpu/drm/drm_mode_config.c
> > >>>>> index afc91447293a..4c1b350ddb95 100644
> > >>>>> --- a/drivers/gpu/drm/drm_mode_config.c
> > >>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
> > >>>>> @@ -581,6 +581,29 @@ static void
> > >> validate_encoder_possible_clones(struct drm_encoder *encoder)
> > >>>>>           encoder->possible_clones, encoder_mask);  }
> > >>>>>
> > >>>>> +static u32 full_crtc_mask(struct drm_device *dev) {
> > >>>>> +    struct drm_crtc *crtc;
> > >>>>> +    u32 crtc_mask = 0;
> > >>>>> +
> > >>>>> +    drm_for_each_crtc(crtc, dev)
> > >>>>> +            crtc_mask |= drm_crtc_mask(crtc);
> > >>>>> +
> > >>>>> +    return crtc_mask;
> > >>>>> +}
> > >>>>> +
> > >>>>> +static void validate_encoder_possible_crtcs(struct drm_encoder
> > >>>>> +*encoder) {
> > >>>>> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
> > >>>>> +
> > >>>>> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> > >>>>> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
> > >>>>> +         "Bogus possible_crtcs: "
> > >>>>> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> > >>>>> +         encoder->base.id, encoder->name,
> > >>>>> +         encoder->possible_crtcs, crtc_mask); }
> > >>>>> +
> > >>>>>  void drm_mode_config_validate(struct drm_device *dev)  {
> > >>>>>      struct drm_encoder *encoder;
> > >>>>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
> > >> drm_device *dev)
> > >>>>>      drm_for_each_encoder(encoder, dev)
> > >>>>>              fixup_encoder_possible_clones(encoder);
> > >>>>>
> > >>>>> -    drm_for_each_encoder(encoder, dev)
> > >>>>> +    drm_for_each_encoder(encoder, dev) {
> > >>>>>              validate_encoder_possible_clones(encoder);
> > >>>>> +            validate_encoder_possible_crtcs(encoder);
> > >>>>> +    }
> > >>>>>  }
> > >>>>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > >>>>> index 3741963b9587..b236269f41ac 100644
> > >>>>> --- a/include/drm/drm_encoder.h
> > >>>>> +++ b/include/drm/drm_encoder.h
> > >>>>> @@ -142,7 +142,7 @@ struct drm_encoder {
> > >>>>>       * the bits for all &drm_crtc objects this encoder can be connected to
> > >>>>>       * before calling drm_dev_register().
> > >>>>>       *
> > >>>>> -     * In reality almost every driver gets this wrong.
> > >>>>> +     * You will get a WARN if you get this wrong in the driver.
> > >>>>>       *
> > >>>>>       * Note that since CRTC objects can't be hotplugged the assigned
> > >> indices
> > >>>>>       * are stable and hence known before registering all objects.
> > >>>>> --
> > >>>>> 2.24.1
> > >>>>>
> > >>>>
> > >>>
> > >>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
> > >>
> > >> Adding amdgpu display folks.
> > >
> > > I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.
> > >
> >
> > So, will this be fixed any time soon? I don't feel qualified writing
> > such a patch but I would obviously be happy to test one.
>
> It's harmless, but I'll send out a patch soon.

I believe the attached patch should fix this.

Alex
Daniel Vetter Dec. 9, 2020, 1:17 p.m. UTC | #8
On Thu, Dec 3, 2020 at 10:31 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Tue, Sep 29, 2020 at 4:04 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Tue, Sep 29, 2020 at 8:31 AM Jan Kiszka <jan.kiszka@web.de> wrote:
> > >
> > > On 10.09.20 20:18, Deucher, Alexander wrote:
> > > > [AMD Public Use]
> > > >
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
> > > >> Daniel Vetter
> > > >> Sent: Monday, September 7, 2020 3:15 AM
> > > >> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
> > > >> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
> > > >> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
> > > >> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
> > > >> gfx@lists.freedesktop.org>; Thomas Zimmermann
> > > >> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
> > > >> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
> > > >>
> > > >> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
> > > >>>
> > > >>> On 11.02.20 18:04, Daniel Vetter wrote:
> > > >>>> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
> > > >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >>>>>
> > > >>>>> WARN if the encoder possible_crtcs is effectively empty or contains
> > > >>>>> bits for non-existing crtcs.
> > > >>>>>
> > > >>>>> v2: Move to drm_mode_config_validate() (Daniel)
> > > >>>>>     Make the docs say we WARN when this is wrong (Daniel)
> > > >>>>>     Extract full_crtc_mask()
> > > >>>>>
> > > >>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > > >>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
> > > >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > >>>>
> > > >>>> When pushing the fixup needs to be applied before the validation
> > > >>>> patch here, because we don't want to anger the bisect gods.
> > > >>>>
> > > >>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > >>>>
> > > >>>> I think with the fixup we should be good enough with the existing
> > > >>>> nonsense in drivers. Fingers crossed.
> > > >>>> -Daniel
> > > >>>>
> > > >>>>
> > > >>>>> ---
> > > >>>>>  drivers/gpu/drm/drm_mode_config.c | 27
> > > >> ++++++++++++++++++++++++++-
> > > >>>>>  include/drm/drm_encoder.h         |  2 +-
> > > >>>>>  2 files changed, 27 insertions(+), 2 deletions(-)
> > > >>>>>
> > > >>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
> > > >>>>> b/drivers/gpu/drm/drm_mode_config.c
> > > >>>>> index afc91447293a..4c1b350ddb95 100644
> > > >>>>> --- a/drivers/gpu/drm/drm_mode_config.c
> > > >>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
> > > >>>>> @@ -581,6 +581,29 @@ static void
> > > >> validate_encoder_possible_clones(struct drm_encoder *encoder)
> > > >>>>>           encoder->possible_clones, encoder_mask);  }
> > > >>>>>
> > > >>>>> +static u32 full_crtc_mask(struct drm_device *dev) {
> > > >>>>> +    struct drm_crtc *crtc;
> > > >>>>> +    u32 crtc_mask = 0;
> > > >>>>> +
> > > >>>>> +    drm_for_each_crtc(crtc, dev)
> > > >>>>> +            crtc_mask |= drm_crtc_mask(crtc);
> > > >>>>> +
> > > >>>>> +    return crtc_mask;
> > > >>>>> +}
> > > >>>>> +
> > > >>>>> +static void validate_encoder_possible_crtcs(struct drm_encoder
> > > >>>>> +*encoder) {
> > > >>>>> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
> > > >>>>> +
> > > >>>>> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
> > > >>>>> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
> > > >>>>> +         "Bogus possible_crtcs: "
> > > >>>>> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
> > > >>>>> +         encoder->base.id, encoder->name,
> > > >>>>> +         encoder->possible_crtcs, crtc_mask); }
> > > >>>>> +
> > > >>>>>  void drm_mode_config_validate(struct drm_device *dev)  {
> > > >>>>>      struct drm_encoder *encoder;
> > > >>>>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
> > > >> drm_device *dev)
> > > >>>>>      drm_for_each_encoder(encoder, dev)
> > > >>>>>              fixup_encoder_possible_clones(encoder);
> > > >>>>>
> > > >>>>> -    drm_for_each_encoder(encoder, dev)
> > > >>>>> +    drm_for_each_encoder(encoder, dev) {
> > > >>>>>              validate_encoder_possible_clones(encoder);
> > > >>>>> +            validate_encoder_possible_crtcs(encoder);
> > > >>>>> +    }
> > > >>>>>  }
> > > >>>>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
> > > >>>>> index 3741963b9587..b236269f41ac 100644
> > > >>>>> --- a/include/drm/drm_encoder.h
> > > >>>>> +++ b/include/drm/drm_encoder.h
> > > >>>>> @@ -142,7 +142,7 @@ struct drm_encoder {
> > > >>>>>       * the bits for all &drm_crtc objects this encoder can be connected to
> > > >>>>>       * before calling drm_dev_register().
> > > >>>>>       *
> > > >>>>> -     * In reality almost every driver gets this wrong.
> > > >>>>> +     * You will get a WARN if you get this wrong in the driver.
> > > >>>>>       *
> > > >>>>>       * Note that since CRTC objects can't be hotplugged the assigned
> > > >> indices
> > > >>>>>       * are stable and hence known before registering all objects.
> > > >>>>> --
> > > >>>>> 2.24.1
> > > >>>>>
> > > >>>>
> > > >>>
> > > >>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
> > > >>
> > > >> Adding amdgpu display folks.
> > > >
> > > > I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.
> > > >
> > >
> > > So, will this be fixed any time soon? I don't feel qualified writing
> > > such a patch but I would obviously be happy to test one.
> >
> > It's harmless, but I'll send out a patch soon.
>
> I believe the attached patch should fix this.

fwiw rb: me

Probably want to add the right Fixes: line too.
-Daniel
Jan Kiszka Dec. 14, 2020, 8:26 p.m. UTC | #9
On 03.12.20 22:30, Alex Deucher wrote:
> On Tue, Sep 29, 2020 at 4:04 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> On Tue, Sep 29, 2020 at 8:31 AM Jan Kiszka <jan.kiszka@web.de> wrote:
>>>
>>> On 10.09.20 20:18, Deucher, Alexander wrote:
>>>> [AMD Public Use]
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>>>> Daniel Vetter
>>>>> Sent: Monday, September 7, 2020 3:15 AM
>>>>> To: Jan Kiszka <jan.kiszka@web.de>; amd-gfx list <amd-
>>>>> gfx@lists.freedesktop.org>; Wentland, Harry <Harry.Wentland@amd.com>;
>>>>> Kazlauskas, Nicholas <Nicholas.Kazlauskas@amd.com>
>>>>> Cc: dri-devel <dri-devel@lists.freedesktop.org>; intel-gfx <intel-
>>>>> gfx@lists.freedesktop.org>; Thomas Zimmermann
>>>>> <tzimmermann@suse.de>; Ville Syrjala <ville.syrjala@linux.intel.com>
>>>>> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
>>>>>
>>>>> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>>>
>>>>>> On 11.02.20 18:04, Daniel Vetter wrote:
>>>>>>> On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote:
>>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>>
>>>>>>>> WARN if the encoder possible_crtcs is effectively empty or contains
>>>>>>>> bits for non-existing crtcs.
>>>>>>>>
>>>>>>>> v2: Move to drm_mode_config_validate() (Daniel)
>>>>>>>>     Make the docs say we WARN when this is wrong (Daniel)
>>>>>>>>     Extract full_crtc_mask()
>>>>>>>>
>>>>>>>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>>>>>>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>
>>>>>>> When pushing the fixup needs to be applied before the validation
>>>>>>> patch here, because we don't want to anger the bisect gods.
>>>>>>>
>>>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>>>>
>>>>>>> I think with the fixup we should be good enough with the existing
>>>>>>> nonsense in drivers. Fingers crossed.
>>>>>>> -Daniel
>>>>>>>
>>>>>>>
>>>>>>>> ---
>>>>>>>>  drivers/gpu/drm/drm_mode_config.c | 27
>>>>> ++++++++++++++++++++++++++-
>>>>>>>>  include/drm/drm_encoder.h         |  2 +-
>>>>>>>>  2 files changed, 27 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/drm_mode_config.c
>>>>>>>> b/drivers/gpu/drm/drm_mode_config.c
>>>>>>>> index afc91447293a..4c1b350ddb95 100644
>>>>>>>> --- a/drivers/gpu/drm/drm_mode_config.c
>>>>>>>> +++ b/drivers/gpu/drm/drm_mode_config.c
>>>>>>>> @@ -581,6 +581,29 @@ static void
>>>>> validate_encoder_possible_clones(struct drm_encoder *encoder)
>>>>>>>>           encoder->possible_clones, encoder_mask);  }
>>>>>>>>
>>>>>>>> +static u32 full_crtc_mask(struct drm_device *dev) {
>>>>>>>> +    struct drm_crtc *crtc;
>>>>>>>> +    u32 crtc_mask = 0;
>>>>>>>> +
>>>>>>>> +    drm_for_each_crtc(crtc, dev)
>>>>>>>> +            crtc_mask |= drm_crtc_mask(crtc);
>>>>>>>> +
>>>>>>>> +    return crtc_mask;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void validate_encoder_possible_crtcs(struct drm_encoder
>>>>>>>> +*encoder) {
>>>>>>>> +    u32 crtc_mask = full_crtc_mask(encoder->dev);
>>>>>>>> +
>>>>>>>> +    WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
>>>>>>>> +         (encoder->possible_crtcs & ~crtc_mask) != 0,
>>>>>>>> +         "Bogus possible_crtcs: "
>>>>>>>> +         "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
>>>>>>>> +         encoder->base.id, encoder->name,
>>>>>>>> +         encoder->possible_crtcs, crtc_mask); }
>>>>>>>> +
>>>>>>>>  void drm_mode_config_validate(struct drm_device *dev)  {
>>>>>>>>      struct drm_encoder *encoder;
>>>>>>>> @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct
>>>>> drm_device *dev)
>>>>>>>>      drm_for_each_encoder(encoder, dev)
>>>>>>>>              fixup_encoder_possible_clones(encoder);
>>>>>>>>
>>>>>>>> -    drm_for_each_encoder(encoder, dev)
>>>>>>>> +    drm_for_each_encoder(encoder, dev) {
>>>>>>>>              validate_encoder_possible_clones(encoder);
>>>>>>>> +            validate_encoder_possible_crtcs(encoder);
>>>>>>>> +    }
>>>>>>>>  }
>>>>>>>> diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
>>>>>>>> index 3741963b9587..b236269f41ac 100644
>>>>>>>> --- a/include/drm/drm_encoder.h
>>>>>>>> +++ b/include/drm/drm_encoder.h
>>>>>>>> @@ -142,7 +142,7 @@ struct drm_encoder {
>>>>>>>>       * the bits for all &drm_crtc objects this encoder can be connected to
>>>>>>>>       * before calling drm_dev_register().
>>>>>>>>       *
>>>>>>>> -     * In reality almost every driver gets this wrong.
>>>>>>>> +     * You will get a WARN if you get this wrong in the driver.
>>>>>>>>       *
>>>>>>>>       * Note that since CRTC objects can't be hotplugged the assigned
>>>>> indices
>>>>>>>>       * are stable and hence known before registering all objects.
>>>>>>>> --
>>>>>>>> 2.24.1
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs):
>>>>>
>>>>> Adding amdgpu display folks.
>>>>
>>>> I took a quick look at this and it looks like we limit the number of crtcs later in the mode init process if the number of physical displays can't actually use more crtcs.  E.g., the physical board configuration would only allow for 3 active displays even if the hardware technically supports 4 crtcs.  I presume that way we can just leave the additional hardware power gated all the time.
>>>>
>>>
>>> So, will this be fixed any time soon? I don't feel qualified writing
>>> such a patch but I would obviously be happy to test one.
>>
>> It's harmless, but I'll send out a patch soon.
>
> I believe the attached patch should fix this.
>

Yep, just booted 5.10, and the warning is gone.

Thanks,
Jan
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index afc91447293a..4c1b350ddb95 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -581,6 +581,29 @@  static void validate_encoder_possible_clones(struct drm_encoder *encoder)
 	     encoder->possible_clones, encoder_mask);
 }
 
+static u32 full_crtc_mask(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+	u32 crtc_mask = 0;
+
+	drm_for_each_crtc(crtc, dev)
+		crtc_mask |= drm_crtc_mask(crtc);
+
+	return crtc_mask;
+}
+
+static void validate_encoder_possible_crtcs(struct drm_encoder *encoder)
+{
+	u32 crtc_mask = full_crtc_mask(encoder->dev);
+
+	WARN((encoder->possible_crtcs & crtc_mask) == 0 ||
+	     (encoder->possible_crtcs & ~crtc_mask) != 0,
+	     "Bogus possible_crtcs: "
+	     "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n",
+	     encoder->base.id, encoder->name,
+	     encoder->possible_crtcs, crtc_mask);
+}
+
 void drm_mode_config_validate(struct drm_device *dev)
 {
 	struct drm_encoder *encoder;
@@ -588,6 +611,8 @@  void drm_mode_config_validate(struct drm_device *dev)
 	drm_for_each_encoder(encoder, dev)
 		fixup_encoder_possible_clones(encoder);
 
-	drm_for_each_encoder(encoder, dev)
+	drm_for_each_encoder(encoder, dev) {
 		validate_encoder_possible_clones(encoder);
+		validate_encoder_possible_crtcs(encoder);
+	}
 }
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 3741963b9587..b236269f41ac 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -142,7 +142,7 @@  struct drm_encoder {
 	 * the bits for all &drm_crtc objects this encoder can be connected to
 	 * before calling drm_dev_register().
 	 *
-	 * In reality almost every driver gets this wrong.
+	 * You will get a WARN if you get this wrong in the driver.
 	 *
 	 * Note that since CRTC objects can't be hotplugged the assigned indices
 	 * are stable and hence known before registering all objects.