Message ID | 1486110513-12130-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, Feb 03, 2017 at 01:58:33PM +0530, Sagar Arun Kamble wrote: > HUC_STATUS, GUC_STATUS, SOFT_SCRATCH registers are read in debugfs > and getparam ioctl. This patch covers those accesses by RPM get/put. > > v2: Covering access in i915_getparam(I915_PARAM_HUC_STATUS) (ChrisW) > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Fiedorowicz, Lukasz <lukasz.fiedorowicz@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ > drivers/gpu/drm/i915/i915_drv.c | 5 ++--- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 3ae0656..639ed12 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2374,7 +2374,9 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data) > seq_printf(m, "\tRSA: offset is %d; size = %d\n", > huc_fw->rsa_offset, huc_fw->rsa_size); > > + intel_runtime_pm_get(dev_priv); > seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2)); > + intel_runtime_pm_put(dev_priv); > > return 0; > } Are you sure that HUC_STATUS2 requires RPM get for reading? I remember trying reading it with device forcefully asleep and it succeed just fine.
On 2/4/2017 7:40 PM, Arkadiusz Hiler wrote: > On Fri, Feb 03, 2017 at 01:58:33PM +0530, Sagar Arun Kamble wrote: >> HUC_STATUS, GUC_STATUS, SOFT_SCRATCH registers are read in debugfs >> and getparam ioctl. This patch covers those accesses by RPM get/put. >> >> v2: Covering access in i915_getparam(I915_PARAM_HUC_STATUS) (ChrisW) >> >> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> >> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> >> Cc: Fiedorowicz, Lukasz <lukasz.fiedorowicz@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ >> drivers/gpu/drm/i915/i915_drv.c | 5 ++--- >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index 3ae0656..639ed12 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -2374,7 +2374,9 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data) >> seq_printf(m, "\tRSA: offset is %d; size = %d\n", >> huc_fw->rsa_offset, huc_fw->rsa_size); >> >> + intel_runtime_pm_get(dev_priv); >> seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2)); >> + intel_runtime_pm_put(dev_priv); >> >> return 0; >> } > Are you sure that HUC_STATUS2 requires RPM get for reading? > > I remember trying reading it with device forcefully asleep and it > succeed just fine. Hi Arek, Faced following warning with device runtime_status=suspended. Also this register lies in media forcewake range and should have similar RPM get/put needs as other registers unless this has special behavior I am not aware of. [ 153.923576] WARNING: CPU: 1 PID: 1859 at drivers/gpu/drm/i915/intel_drv.h:1698 gen9_decoupled_read32+0x1a1/0x1c0 [i915] [ 153.923578] Device suspended during HW access [ 153.923580] Modules linked in: spi_pxa2xx_platform 8250_dw i2c_designware_platform i2c_designware_core intel_rapl intel_telemetry_debugfs intel_telemetry_pltdrv intel_punit_ipc intel_telemetry_core x86_pkg_temp_thermal binfmt_misc coretemp kvm_intel snd_soc_skl kvm snd_soc_rt298 snd_soc_skl_ipc snd_soc_rt286 snd_soc_sst_ipc snd_soc_rl6347a snd_soc_sst_dsp snd_hda_ext_core snd_soc_sst_match irqbypass nls_iso8859_1 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core snd_soc_core snd_hwdep aesni_intel snd_compress aes_x86_64 ac97_bus crypto_simd snd_pcm_dmaengine glue_helper cryptd snd_pcm intel_rapl_perf snd_seq_midi snd_seq_midi_event wdat_wdt snd_rawmidi input_leds serio_raw snd_seq pwm_lpss_pci pwm_lpss snd_seq_device snd_timer idma64 [ 153.923671] virt_dma intel_lpss_pci intel_lpss snd hid_sensor_accel_3d hid_sensor_magn_3d hid_sensor_als hid_sensor_gyro_3d hid_sensor_incl_3d hid_sensor_rotation hid_sensor_press hid_sensor_trigger mei_me industrialio_triggered_buffer kfifo_buf hid_sensor_iio_common industrialio mei soundcore shpchp rfkill_gpio soc_button_array intel_vbtn tpm_crb ucsi intel_hid intel_pmc_ipc sparse_keymap mac_hid autofs4 hid_generic usbhid hid_sensor_custom hid_sensor_hub intel_ishtp_hid i915(E) drm_kms_helper syscopyarea psmouse igb sysfillrect sdhci_pci sysimgblt fb_sys_fops sdhci dca ptp drm pps_core ahci intel_ish_ipc i2c_algo_bit libahci intel_ishtp i2c_hid hid video fjes pinctrl_broxton pinctrl_intel [ 153.923768] CPU: 1 PID: 1859 Comm: cat Tainted: G U W E 4.10.0-rc6-ww5.5mainlinestaging #12 [ 153.923771] Hardware name: Intel Corp. Broxton P/Apollolake RVP1C, BIOS APLIRVPA.X64.0151.B24.1609210905 09/21/2016 [ 153.923773] Call Trace: [ 153.923785] dump_stack+0x63/0x90 [ 153.923791] __warn+0xd1/0xf0 [ 153.923795] warn_slowpath_fmt+0x4f/0x60 [ 153.923803] ? seq_vprintf+0x35/0x50 [ 153.923893] gen9_decoupled_read32+0x1a1/0x1c0 [i915] [ 153.923969] i915_huc_load_status_info+0x132/0x150 [i915] [ 153.923974] seq_read+0x119/0x3e0 [ 153.923981] full_proxy_read+0x51/0x80 [ 153.923988] __vfs_read+0x28/0x130 [ 153.923993] ? security_file_permission+0x9d/0xc0 [ 153.923998] ? rw_verify_area+0x4e/0xb0 [ 153.924003] vfs_read+0x93/0x130 [ 153.924009] SyS_read+0x46/0xa0 [ 153.924015] entry_SYSCALL_64_fastpath+0x1e/0xad [ 153.924019] RIP: 0033:0x7fe18cd44680 [ 153.924022] RSP: 002b:00007ffe7d991eb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 [ 153.924027] RAX: ffffffffffffffda RBX: 00007fe18d011b20 RCX: 00007fe18cd44680 [ 153.924030] RDX: 0000000000020000 RSI: 00007fe18d1fb000 RDI: 0000000000000003 [ 153.924032] RBP: 0000000000021010 R08: ffffffffffffffff R09: 0000000000000000 [ 153.924035] R10: 000000000000037b R11: 0000000000000246 R12: 0000000000022000 [ 153.924037] R13: 00007fe18d011b78 R14: 0000000000001000 R15: 0000000000020000 [ 153.924042] ---[ end trace 6529f5b46f5753d0 ]--- [ 153.974135] [drm:__gen9_decoupled_mmio_access [i915]] *ERROR* Decoupled MMIO wait timed out >
On Mon, Feb 06, 2017 at 10:34:31AM +0530, Kamble, Sagar A wrote: > > > On 2/4/2017 7:40 PM, Arkadiusz Hiler wrote: > > On Fri, Feb 03, 2017 at 01:58:33PM +0530, Sagar Arun Kamble wrote: > > > HUC_STATUS, GUC_STATUS, SOFT_SCRATCH registers are read in debugfs > > > and getparam ioctl. This patch covers those accesses by RPM get/put. > > > > > > v2: Covering access in i915_getparam(I915_PARAM_HUC_STATUS) (ChrisW) > > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> > > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > > Cc: Fiedorowicz, Lukasz <lukasz.fiedorowicz@intel.com> > > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ > > > drivers/gpu/drm/i915/i915_drv.c | 5 ++--- > > > 2 files changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > > index 3ae0656..639ed12 100644 > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > @@ -2374,7 +2374,9 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data) > > > seq_printf(m, "\tRSA: offset is %d; size = %d\n", > > > huc_fw->rsa_offset, huc_fw->rsa_size); > > > + intel_runtime_pm_get(dev_priv); > > > seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2)); > > > + intel_runtime_pm_put(dev_priv); > > > return 0; > > > } > > Are you sure that HUC_STATUS2 requires RPM get for reading? > > > > I remember trying reading it with device forcefully asleep and it > > succeed just fine. > Hi Arek, > > Faced following warning with device runtime_status=suspended. > Also this register lies in media forcewake range and should have similar RPM > get/put needs as other registers unless this has special behavior I am not > aware of. Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
On 03/02/2017 08:54, Patchwork wrote: > == Series Details == > > Series: series starting with [v2,1/1] drm/i915: Do RPM Wake during GuC/HuC status read > URL : https://patchwork.freedesktop.org/series/19037/ > State : success > > == Summary == > > Series 19037v1 Series without cover letter > https://patchwork.freedesktop.org/api/1.0/series/19037/revisions/1/mbox/ > > > fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 > fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 > fi-bxt-j4205 total:247 pass:225 dwarn:0 dfail:0 fail:0 skip:22 > fi-bxt-t5700 total:78 pass:65 dwarn:0 dfail:0 fail:0 skip:12 > fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 > fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 > fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 > fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 > fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 > fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 > fi-kbl-7500u total:247 pass:224 dwarn:0 dfail:0 fail:2 skip:21 > fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 > fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 > fi-skl-6700k total:247 pass:222 dwarn:4 dfail:0 fail:0 skip:21 > fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 > fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 > fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 > > 0f01216949002d20b9dc6d300c82df5ffa59e9a7 drm-tip: 2017y-02m-02d-19h-49m-15s UTC integration manifest > 87bbc3e drm/i915: Do RPM Wake during GuC/HuC status read Pushed, thanks for the patch and review! Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 3ae0656..639ed12 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2374,7 +2374,9 @@ static int i915_huc_load_status_info(struct seq_file *m, void *data) seq_printf(m, "\tRSA: offset is %d; size = %d\n", huc_fw->rsa_offset, huc_fw->rsa_size); + intel_runtime_pm_get(dev_priv); seq_printf(m, "\nHuC status 0x%08x:\n", I915_READ(HUC_STATUS2)); + intel_runtime_pm_put(dev_priv); return 0; } @@ -2406,6 +2408,8 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data) seq_printf(m, "\tRSA: offset is %d; size = %d\n", guc_fw->rsa_offset, guc_fw->rsa_size); + intel_runtime_pm_get(dev_priv); + tmp = I915_READ(GUC_STATUS); seq_printf(m, "\nGuC status 0x%08x:\n", tmp); @@ -2419,6 +2423,8 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data) for (i = 0; i < 16; i++) seq_printf(m, "\t%2d: \t0x%x\n", i, I915_READ(SOFT_SCRATCH(i))); + intel_runtime_pm_put(dev_priv); + return 0; } diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 4790182..cd228b3 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -319,10 +319,9 @@ static int i915_getparam(struct drm_device *dev, void *data, value = INTEL_INFO(dev_priv)->sseu.min_eu_in_pool; break; case I915_PARAM_HUC_STATUS: - /* The register is already force-woken. We dont need - * any rpm here - */ + intel_runtime_pm_get(dev_priv); value = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED; + intel_runtime_pm_put(dev_priv); break; case I915_PARAM_MMAP_GTT_VERSION: /* Though we've started our numbering from 1, and so class all
HUC_STATUS, GUC_STATUS, SOFT_SCRATCH registers are read in debugfs and getparam ioctl. This patch covers those accesses by RPM get/put. v2: Covering access in i915_getparam(I915_PARAM_HUC_STATUS) (ChrisW) Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Fiedorowicz, Lukasz <lukasz.fiedorowicz@intel.com> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ drivers/gpu/drm/i915/i915_drv.c | 5 ++--- 2 files changed, 8 insertions(+), 3 deletions(-)