[Intel-gfx] [PATCH v2 4/6] drm/i915/guc: remove writes to GEN8_DRBREG registers

[Dave Gordon]

On 13/06/16 10:35, Tvrtko Ursulin wrote:
>
> On 10/06/16 17:50, Dave Gordon wrote:
>> These registers are not actually writable by the CPU; only the GuC can
>> actually program them. So let's not do writes that have no effect.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 5 -----
>>   1 file changed, 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index e198599..45b33f8 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -197,14 +197,9 @@ static void guc_disable_doorbell(struct intel_guc
>> *guc,
>>
>>       doorbell->db_status = GUC_DOORBELL_DISABLED;
>>
>> -    I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID);
>> -
>>       value = I915_READ(drbreg);
>>       WARN_ON((value & GEN8_DRB_VALID) != 0);
>>
>> -    I915_WRITE(GEN8_DRBREGU(client->doorbell_id), 0);
>> -    I915_WRITE(drbreg, 0);
>> -
>>       /* XXX: wait for any interrupts */
>>       /* XXX: wait for workqueue to drain */
>>   }
>>
>
> I have to trust you on this one. :)
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Regards,
> Tvrtko

I found an example of what can occasionally happen during 
resume-from-disk with the old code:

[  407.811985] [drm:intel_guc_setup] GuC fw status: path 
i915/skl_guc_ver6_1.bin, fetch SUCCESS, load SUCCESS
[  407.811988] [drm:intel_guc_setup] GuC fw status: fetch SUCCESS, load 
PENDING
[  407.811989] ------------[ cut here ]------------
[  407.812006] WARNING: CPU: 2 PID: 1501 at 
/usr2/dsgordon/Source/drm-intel-nightly/drm-intel/drivers/gpu/drm/i915/i915_guc_submission.c:258 
guc_client_free+0x190/0x1a0 [i915]
[  407.812007] WARN_ON((value & (1<<0)) != 0)

In other words, we have just "written" this register with the VALID bit 
clear, but when we read it back it's NOT clear, showing that CPU writes 
really don't have any effect.

[  407.812017] Modules linked in: nls_iso8859_1 nls_cp437 vfat fat 
snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core 
x86_pkg_temp_thermal coretemp snd_pcm input_leds led_class snd_seq 
snd_seq_device snd_timer snd soundcore serio_raw acpi_pad tpm_tis 
autofs4 hid_generic usbhid hid i915 e1000e i2c_algo_bit drm_kms_helper 
ptp syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse pps_core drm video
[  407.812018] CPU: 2 PID: 1501 Comm: kworker/u16:38 Tainted: G     U 
        4.7.0-rc2-dsg-00656-g41795e2-dsg-work-080 #1501
[  407.812019] Hardware name: Intel Corporation Skylake Client 
platform/Skylake AIO DDR3L RVP10, BIOS SKLSE2R1.R00.X100.B01.1509220551 
09/22/2015
[  407.812022] Workqueue: events_unbound async_run_entry_fn
[  407.812023]  0000000000000000 ffff880165c1bae8 ffffffff81331caf 
ffff880165c1bb38
[  407.812024]  0000000000000000 ffff880165c1bb28 ffffffff8105aed1 
0000010200000006
[  407.812025]  ffff88016675f200 ffff8801667c0000 0000000000001000 
0000000000000000
[  407.812025] Call Trace:
[  407.812028]  [<ffffffff81331caf>] dump_stack+0x4d/0x6e
[  407.812030]  [<ffffffff8105aed1>] __warn+0xd1/0xf0
[  407.812031]  [<ffffffff8105af3f>] warn_slowpath_fmt+0x4f/0x60
[  407.812044]  [<ffffffffa01c6e20>] ? chv_write16+0x390/0x390 [i915]
[  407.812056]  [<ffffffffa01cad60>] guc_client_free+0x190/0x1a0 [i915]
[  407.812067]  [<ffffffffa01c6e20>] ? chv_write16+0x390/0x390 [i915]
[  407.812078]  [<ffffffffa01cb6eb>] 
i915_guc_submission_disable+0x4b/0x60 [i915]
[  407.812088]  [<ffffffffa01cb734>] i915_guc_submission_init+0x34/0x3a0 
[i915]
[  407.812098]  [<ffffffffa01c98fd>] intel_guc_setup+0x9d/0x820 [i915]
[  407.812108]  [<ffffffffa01c6e20>] ? chv_write16+0x390/0x390 [i915]
[  407.812118]  [<ffffffffa01b8abb>] ? 
intel_mocs_init_l3cc_table+0xbb/0x110 [i915]
[  407.812128]  [<ffffffffa01a761c>] i915_gem_init_hw+0x10c/0x2a0 [i915]
[  407.812130]  [<ffffffff81375fb0>] ? pci_pm_suspend_noirq+0x190/0x190
[  407.812136]  [<ffffffffa0168086>] i915_drm_resume+0x86/0x180 [i915]
[  407.812143]  [<ffffffffa01681a7>] i915_pm_resume+0x27/0x30 [i915]
[  407.812149]  [<ffffffffa01681be>] i915_pm_restore+0xe/0x10 [i915]
[  407.812150]  [<ffffffff81376029>] pci_pm_restore+0x79/0xb0
[  407.812152]  [<ffffffff81450e7e>] dpm_run_callback+0x4e/0x130
[  407.812153]  [<ffffffff81451403>] device_resume+0xd3/0x1f0
[  407.812154]  [<ffffffff8145153d>] async_resume+0x1d/0x50
[  407.812154]  [<ffffffff81079bf8>] async_run_entry_fn+0x48/0x150
[  407.812156]  [<ffffffff81071d28>] process_one_work+0x148/0x3f0
[  407.812157]  [<ffffffff810720fb>] worker_thread+0x12b/0x490
[  407.812159]  [<ffffffff81071fd0>] ? process_one_work+0x3f0/0x3f0
[  407.812159]  [<ffffffff81077159>] kthread+0xc9/0xe0
[  407.812162]  [<ffffffff816c46cf>] ret_from_fork+0x1f/0x40
[  407.812162]  [<ffffffff81077090>] ? kthread_park+0x60/0x60
[  407.812163] ---[ end trace 7dc15bdf4dd23983 ]---
[  407.815449] [drm:guc_ucode_xfer_dma] DMA status 0x10, GuC status 
0x8002f0ec
[  407.815449] [drm:guc_ucode_xfer_dma] returning 0
[  407.815450] [drm:intel_guc_setup] GuC fw status: fetch SUCCESS, load 
SUCCESS

.Dave.