Re: [Intel-gfx] memcontrol.c BUG
On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1165369 > > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2 > mapcount:0 mapping: (null) index:0x0 > Nov 18 09:23:22 elissa.gathman.org kernel: page flags: > 0x80090029(locked|uptodate|lru|swapcache|swapbacked) > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because: > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage)) > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here > ] > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at mm/memcontrol.c:6733! > Nov 18 09:23:23 elissa.gathman.org kernel: invalid opcode: [#1] SMP > Nov 18 09:23:23 elissa.gathman.org kernel: Modules linked in: tcp_lp > vfat fat fuse tun ccm bnep bluetooth ip6t_rpfilter ip6t_REJECT > xt_conntrack ebtable_nat ebtable_broute bridge stp llc ebtable_filter > ebtables ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 > ip6table_mangle ip6table_security ip6table_raw ip6table_filter > ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 > nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw > snd_hda_codec_idt snd_hda_codec_hdmi snd_hda_codec_generic mmc_block > snd_hda_intel snd_hda_controller iTCO_wdt snd_hda_codec > iTCO_vendor_support dell_wmi sdhci_pci gpio_ich sparse_keymap arc4 > joydev iwl3945 snd_hwdep serio_raw sdhci snd_seq dell_laptop iwlegacy > dcdbas mac80211 coretemp i2c_i801 r592 mmc_core snd_seq_device snd_pcm > cfg80211 memstick rfkill snd_timer lpc_ich > Nov 18 09:23:23 elissa.gathman.org kernel: snd soundcore wmi > acpi_cpufreq hid_logitech_dj firewire_ohci firewire_core ata_generic > i915 pata_acpi crc_itu_t i2c_algo_bit sky2 drm_kms_helper drm video > Nov 18 09:23:23 elissa.gathman.org kernel: CPU: 0 PID: 966 Comm: Xorg > Tainted: GW 3.17.2-200.fc20.i686 #1 > Nov 18 09:23:23 elissa.gathman.org kernel: Hardware name: Dell Inc. > Inspiron 1525 /0U990C, BIOS A16 10/16/2008 > Nov 18 09:23:24 elissa.gathman.org kernel: task: e88adf80 ti: e88a2000 > task.ti: e88a2000 > Nov 18 09:23:24 elissa.gathman.org kernel: EIP: 0060:[] > EFLAGS: 00013282 CPU: 0 > Nov 18 09:23:24 elissa.gathman.org kernel: EIP is at > mem_cgroup_migrate+0x14b/0x180 > Nov 18 09:23:24 elissa.gathman.org kernel: EAX: EBX: f54eec40 > ECX: f73d8ac8 EDX: > Nov 18 09:23:24 elissa.gathman.org kernel: ESI: f5e36a40 EDI: c0cd3a40 > EBP: e88a3b9c ESP: e88a3b84 > Nov 18 09:23:24 elissa.gathman.org kernel: DS: 007b ES: 007b FS: 00d8 > GS: 00e0 SS: 0068 > Nov 18 09:23:24 elissa.gathman.org kernel: CR0: 80050033 CR2: b69e6000 > CR3: 2f3ab000 CR4: 07d0 > Nov 18 09:23:24 elissa.gathman.org kernel: Stack: > Nov 18 09:23:24 elissa.gathman.org kernel: c0d215c0 f54eec40 2a278f72 > f54eec40 c0cd3a40 e88a3bc8 c053d71b > Nov 18 09:23:24 elissa.gathman.org kernel: f54eec40 e88a3c20 8537 > fffba000 c0cd3a40 f5e36a40 8537 ef3df828 > Nov 18 09:23:24 elissa.gathman.org kernel: ffef e88a3c34 c053db59 > e88a3c04 000214de ef3df8f8 e88adf80 e88a3c48 > Nov 18 09:23:24 elissa.gathman.org kernel: Call Trace: > Nov 18 09:23:25 elissa.gathman.org kernel: [] > shmem_replace_page.isra.28+0x11b/0x200 > Nov 18 09:23:25 elissa.gathman.org kernel: [] > shmem_getpage_gfp+0x239/0x770 > Nov 18 09:23:25 elissa.gathman.org kernel: [] > shmem_read_mapping_page_gfp+0x3f/0x70 > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? sg_kfree+0x30/0x30 > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_object_get_pages_gtt+0x141/0x2c0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > nommu_map_sg+0x40/0xb0 > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > intel_gtt_insert_sg_entries+0x72/0xa0 > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_object_get_pages+0x66/0xa0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_object_pin+0x3ed/0x6e0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_execbuffer_reserve_vma.isra.11+0x75/0x130 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_execbuffer_reserve+0x2a5/0x2d0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_do_execbuffer.isra.18+0x51e/0x11d0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > i915_gem_object_set_to_gtt_domain+0xfc/0x1b0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > i915_gem_object_ggtt_unpin+0x15/0x90 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > i915_gem_execbuffer2+0x59/0x2b0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > i915_gem_execbuffer2+0x8b/0x2b0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > i915_gem_execbuffer+0x4e0/0x4e0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] > drm_ioctl+0x1cf/0x520 [drm] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > i915_gem_execbuffer+0x4e0/0x4e0 [i915] > Nov 18 09:23:25 elissa.gathman.org kernel: [] ? > timerqueue_add+0x50/0xb0 > Nov 18 09:23:25 elissa.gathma
[Intel-gfx] [PATCH] tools/intel_reg_read: Adding the reg offset for VLV and CHT
From: meghanelogal
For VLV and CHT for each register access we need to add base offset of
0x18.
Signed-off-by: meghanelogal
---
tools/intel_reg_read.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
index 3b91291..c550b02 100644
--- a/tools/intel_reg_read.c
+++ b/tools/intel_reg_read.c
@@ -48,11 +48,13 @@ static void bit_decode(uint32_t reg)
static void dump_range(uint32_t start, uint32_t end)
{
- int i;
-
+ int i, reg = 0;
+ struct pci_device *dev = intel_get_pci_device();
+ if (IS_CHERRYVIEW(dev->device_id) || IS_VALLEYVIEW(dev->device_id))
+ reg = 0x18;
for (i = start; i < end; i += 4)
printf("0x%X : 0x%X\n", i,
- *(volatile uint32_t *)((volatile char*)mmio + i));
+ *(volatile uint32_t *)((volatile char*)mmio + i + reg));
}
static void usage(char *cmdname)
@@ -129,11 +131,17 @@ int main(int argc, char** argv)
sscanf(argv[i], "0x%x", ®);
dump_range(reg, reg + (dwords * 4));
- if (decode_bits)
- bit_decode(*(volatile uint32_t *)((volatile
char*)mmio + reg));
+ if (decode_bits) {
+ struct pci_device *dev = intel_get_pci_device();
+ if (IS_CHERRYVIEW(dev->device_id) ||
+ IS_VALLEYVIEW(dev->device_id)) {
+ reg += 0x18;
+ bit_decode(*(volatile uint32_t *)
+ ((volatile char*)mmio + reg));
+ }
+ }
}
}
-
intel_register_access_fini();
out:
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/mst: fix recursive sleep warning on qlock
On Wed, Jan 28, 2015 at 04:37:01PM +1300, Dave Airlie wrote:
> From: Dave Airlie
>
> With drm-next, we can get a backtrace like below,
> this is due to the callback checking the txmsg state taking
> the mutex, which can cause a sleep inside a sleep,
>
> Fix this my creating a spinlock protecting the txmsg state
> and locking it in appropriate places.
>
> : [ cut here ]
> : WARNING: CPU: 3 PID: 252 at kernel/sched/core.c:7300
> __might_sleep+0xbd/0xd0()
> : do not call blocking ops when !TASK_RUNNING; state=2 set at
> [] prepare_to_wait_event+0x5d/0x110
> : Modules linked in: i915 i2c_algo_bit drm_kms_helper drm e1000e ptp pps_core
> i2c_core video
> : CPU: 3 PID: 252 Comm: kworker/3:2 Not tainted 3.19.0-rc5+ #18
> : Hardware name: LENOVO 20ARS25701/20ARS25701, BIOS GJET72WW (2.22 )
> 02/21/2014
> : Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper]
> : 81a4027c 88030a763af8 81752699
> : 88030a763b48 88030a763b38 810974ca 88030a763b38
> : 81a41123 026d 0fa0
> : Call Trace:
> : [] dump_stack+0x4c/0x65
> : [] warn_slowpath_common+0x8a/0xc0
> : [] warn_slowpath_fmt+0x46/0x50
> : [] ? prepare_to_wait_event+0x5d/0x110
> : [] ? prepare_to_wait_event+0x5d/0x110
> : [] __might_sleep+0xbd/0xd0
> : [] mutex_lock_nested+0x2e/0x3e0
> : [] ? prepare_to_wait_event+0x98/0x110
> : [] drm_dp_mst_wait_tx_reply+0xa7/0x220 [drm_kms_helper]
> : [] ? wait_woken+0xc0/0xc0
> : [] drm_dp_send_link_address+0x61/0x240 [drm_kms_helper]
> : [] ? process_one_work+0x14f/0x530
> : [] drm_dp_check_and_send_link_address+0x8d/0xa0
> [drm_kms_helper]
> : [] drm_dp_mst_link_probe_work+0x1c/0x20 [drm_kms_helper]
> : [] process_one_work+0x1c6/0x530
> : [] ? process_one_work+0x14f/0x530
> : [] worker_thread+0x6b/0x490
> : [] ? rescuer_thread+0x350/0x350
> : [] kthread+0x10a/0x120
> : [] ? _raw_spin_unlock_irq+0x30/0x50
> : [] ? kthread_create_on_node+0x220/0x220
> : [] ret_from_fork+0x7c/0xb0
> : [] ? kthread_create_on_node+0x220/0x220
> : ---[ end trace bb11c9634a7217c6 ]---
>
> Signed-off-by: Dave Airlie
Imo much simpler with the below diff. Not tested though.
-Daniel
--
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9a5b68717ec8..379ab4555756 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -733,10 +733,14 @@ static bool check_txmsg_state(struct
drm_dp_mst_topology_mgr *mgr,
struct drm_dp_sideband_msg_tx *txmsg)
{
bool ret;
- mutex_lock(&mgr->qlock);
+
+ /*
+* All updates to txmsg->state are protected by mgr->qlock, and the two
+* cases we check here are terminal states. For those the barriers
+* provided by the wake_up/wait_event pair are enough.
+*/
ret = (txmsg->state == DRM_DP_SIDEBAND_TX_RX ||
txmsg->state == DRM_DP_SIDEBAND_TX_TIMEOUT);
- mutex_unlock(&mgr->qlock);
return ret;
}
@@ -1363,12 +1367,13 @@ static int process_single_tx_qlock(struct
drm_dp_mst_topology_mgr *mgr,
return 0;
}
-/* must be called holding qlock */
static void process_single_down_tx_qlock(struct drm_dp_mst_topology_mgr *mgr)
{
struct drm_dp_sideband_msg_tx *txmsg;
int ret;
+ WARN_ON(!mutex_is_locked(&mgr->qlock));
+
/* construct a chunk from the first msg in the tx_msg queue */
if (list_empty(&mgr->tx_msg_downq)) {
mgr->tx_down_in_progress = false;
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in this WARN at boot (and pretty frequently afterwards): WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 gen6_set_rps+0x371/0x3c0() WARN_ON(val > dev_priv->rps.max_freq_softlimit) Modules linked in: CPU: 0 PID: 989 Comm: kworker/0:2 Not tainted 3.19.0-rc6 #31 Hardware name: LENOVO 20A7002WUS/20A7002WUS, BIOS GRET38WW (1.15 ) 05/29/2014 Workqueue: events intel_gen6_powersave_work 81a82dd0 817f099e 88021451bd28 8107d107 8802148e 0022 8802148e86f0 88021498f000 0004 8107d185 81a83448 Call Trace: [] ? dump_stack+0x40/0x50 [] ? warn_slowpath_common+0x77/0xb0 [] ? warn_slowpath_fmt+0x45/0x50 [] ? gen6_set_rps+0x371/0x3c0 [] ? intel_gen6_powersave_work+0x780/0x1180 [] ? process_one_work+0x130/0x350 [] ? worker_thread+0x114/0x450 [] ? rescuer_thread+0x2f0/0x2f0 [] ? kthread+0xbc/0xe0 [] ? kthread_create_on_node+0x170/0x170 [] ? ret_from_fork+0x7c/0xb0 [] ? kthread_create_on_node+0x170/0x170 ---[ end trace c3ac159c87b9b234 ]--- I bisected this back to: commit 93ee29203f506582cca2bcec5f05041526d9ab0a Author: Tom O'Rourke Date: Wed Nov 19 14:21:52 2014 -0800 drm/i915: Use efficient frequency for HSW/BDW Added gen6_init_rps_frequencies() to initialize the rps frequency values. This function replaces parse_rp_state_cap(). In addition to reading RPn, RP0, and RP1 from RP_STATE_CAP register, the new function reads efficient frequency (aka RPe) from pcode for Haswell and Broadwell and sets the turbo softlimits. The turbo minimum frequency softlimit is set to RPe for Haswell and Broadwell and to RPn otherwise. For RPe, the efficiency is based on the frequency/power ratio (MHz/W); this is considering GT power and not package power. The efficent frequency is the highest frequency for which the frequency/power ratio is within some threshold of the highest frequency/power ratio. A fixed decrease in frequency results in smaller decrease in power at frequencies less than RPe than at frequencies above RPe. v2: Following suggestions from Chris Wilson and Daniel Vetter to extend and rename parse_rp_state_cap and to open-code a poorly named function. Signed-off-by: Tom O'Rourke Reviewed-by: Chris Wilson [danvet: Remove unused variables.] Signed-off-by: Daniel Vetter I'm not at all familiar with this hardware, but I took a quick look into what changed with this commit for my laptop. Before the commit, rps.min_freq_softlimit is 4 (from rps.min_freq) and rps.max_freq_softlimit is 22. After the commit, rps.min_freq_softlimit is set to the rps.efficient_freq value read from pcode, which is 34 on my laptop. So later when gen6_set_rps() is called with rps.min_freq_softlimit that warning is hit. Any thoughts? It certainly seems fishy that this commit causes rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drivers: gpu: drm: i915: intel_fifo_underrun.c: Fix a typo in comment
The comment for intel_cpu_fifo_underrun_irq_handler() is not consistent with the code and the rest of the comment for this routine. This patch fixes this typo in comment. Signed-off-by: Kumar Amit Mehta --- drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c index 77af512..04e248d 100644 --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c @@ -341,7 +341,7 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv, } /** - * intel_pch_fifo_underrun_irq_handler - handle PCH fifo underrun interrupt + * intel_cpu_fifo_underrun_irq_handler - handle CPU fifo underrun interrupt * @dev_priv: i915 device instance * @pipe: (CPU) pipe to set state for * -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
Dear list members, I suffer from highly disturbing artefacts with the Intel HD 4400 chip set on thw 2560x1440 display (Sharp LQ133T1JW19) of my Fujitsu Lifebook S904. The effects are hard to describe, therefore I have uploaded a video to https://www.dropbox.com/sh/9wh0nzvp0w1phxz/AAAsvAqHN8ApXzor58FmGbwLa?dl=0. I can distinguish 2 effects that are possibly related to each other: 1. Maximized windows under GNOME3 (fully maximized or half-maximized on the right edge of the screen) fill the screen vertically with a single color (usually grey). Half-maximzed windows on the left edge are not affected. The effect is only seen if the Laptop screen is the main monitor in GNOME (showing the panel on top). Some applications, which start in maximized mode and can't be resized, are unusable on this screen. 2. Artefacts appear when windows are placed on the right and bottom areas of the screen. The artefacts look as if pixels extend vertically from their horizontal position to the bottom edge of the screen (resulting in vertical lines). The strength of the effect depends on the content of the windows that are visible. The effect "extends" from the window causing it to other windows and the background. The effect is not seen in screenshots (screenshot looks fine although the screen was unreadable at the time I took it). 1.) is observed only under GNOME, but I see 2.) on KDE and LXDE, too. I also tried various distributions. Most of my experiments have been done on OpenSUSE 13.2, kernel-vanilla-3.19.rc5, xorg 7.6_1.16.1-5.1, intel 2.99.916, libdrm_intel1-2.4.58-1.1, Mesa 10.3.0-91.3.2. But I see the artefacts also on Fedora 21 and Ubuntu 14.04. As a matter of fact, I tried the modesetting driver and even fbdev and observed similar effects, too. Thus i915 may actually not be the culprit here, but could I figure no other place to ask for help than this mailing list. I have also experimented with resolutions and refresh rates. Only at 1024x768 and lower I was unable to reproduce the problem. Any help would be highly appreciated. I am highly willing to do any testing and/or debugging that may be necessary to find the problem. Best regards Martin martin@artemis:~> xrandr --verbose Screen 0: minimum 8 x 8, current 2560 x 1440, maximum 32767 x 32767 eDP1 connected primary 2560x1440+0+0 (0x49) normal (normal left inverted right x axis y axis) 294mm x 165mm Identifier: 0x43 Timestamp: 19447 Subpixel: unknown Gamma: 1.0:1.0:1.0 Brightness: 1.0 Clones: CRTC: 0 CRTCs: 0 1 2 Transform: 1.00 0.00 0.00 0.00 1.00 0.00 0.00 0.00 1.00 filter: _MUTTER_PRESENTATION_OUTPUT: 0 EDID: 00004d101214 24170104a51d1178068800a4564d9a26 0d50540001010101010101010101 010101010101735f00a0a0a03a503020 e50026a51018454c00a0a0a03a50 3020e50026a51018aa3f00a0a0a0 3a503020e50026a5101800fc 004c513154314a5731390a2000b6 BACKLIGHT: 892 range: (0, 892) Backlight: 892 range: (0, 892) scaling mode: Full aspect supported: None, Full, Center, Full aspect Broadcast RGB: Automatic supported: Automatic, Full, Limited 16:235 audio: auto supported: force-dvi, off, auto, on 2560x1440 (0x49) 244.350MHz -HSync -VSync *current +preferred h: width 2560 start 2608 end 2640 total 2720 skew0 clock 89.83KHz v: height 1440 start 1454 end 1459 total 1498 clock 59.97Hz 2560x1440 (0xae) 195.250MHz -HSync -VSync h: width 2560 start 2608 end 2640 total 2720 skew0 clock 71.78KHz v: height 1440 start 1454 end 1459 total 1498 clock 47.92Hz 2560x1440 (0xaf) 162.980MHz -HSync -VSync h: width 2560 start 2608 end 2640 total 2720 skew0 clock 59.92KHz v: height 1440 start 1454 end 1459 total 1498 clock 40.00Hz 1920x1440 (0xb0) 234.000MHz -HSync +VSync h: width 1920 start 2048 end 2256 total 2600 skew0 clock 90.00KHz v: height 1440 start 1441 end 1444 total 1500 clock 60.00Hz 1856x1392 (0xb1) 218.300MHz -HSync +VSync h: width 1856 start 1952 end 2176 total 2528 skew0 clock 86.35KHz v: height 1392 start 1393 end 1396 total 1439 clock 60.01Hz 1792x1344 (0xb2) 204.800MHz -HSync +VSync h: width 1792 start 1920 end 2120 total 2448 skew0 clock 83.66KHz v: height 1344 start 1345 end 1348 total 1394 clock 60.01Hz 1600x1200 (0xb3) 162.000MHz +HSync +VSync h: width 1600 start 1664 end 1856 total 2160 skew0 clock 75.00KHz v: height 1200 start 1201 end 1204 total 1250 clock 60.00Hz 14
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Fallback to using CPU relocations for large batch buffers
On Tue, Jan 27, 2015 at 09:43:00PM +, Chris Wilson wrote:
> On Tue, Jan 27, 2015 at 04:09:37PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 14, 2015 at 11:20:56AM +, Chris Wilson wrote:
> > > static int
> > > i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> > > struct intel_engine_cs *ring,
> > > @@ -536,14 +589,21 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma
> > > *vma,
> > > int ret;
> > >
> > > flags = 0;
> > > - if (entry->flags & __EXEC_OBJECT_NEEDS_MAP)
> > > - flags |= PIN_GLOBAL | PIN_MAPPABLE;
> > > - if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
> > > - flags |= PIN_GLOBAL;
> > > - if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS)
> > > - flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> > > + if (!drm_mm_node_allocated(&vma->node)) {
> > > + if (entry->flags & __EXEC_OBJECT_NEEDS_MAP)
> > > + flags |= PIN_GLOBAL | PIN_MAPPABLE;
> > > + if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
> > > + flags |= PIN_GLOBAL;
> > > + if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS)
> > > + flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> > > + }
> >
> > Hm, aren't we always calling reserve un buffers we know we've just
> > unbound? Keeping the flags computation would at least be a good selfcheck
> > about the consistency of our placing decisions, so I'd like to keep it.
I still think this isn't required, even with the ping-pong preventer below
kept. Maybe add a WARN_ON(drm_mm_node_allocated); just for paranoia
instead?
> >
> > >
> > > ret = i915_gem_object_pin(obj, vma->vm, entry->alignment, flags);
> > > + if ((ret == -ENOSPC || ret == -E2BIG) &&
> > > + only_mappable_for_reloc(entry->flags))
> > > + ret = i915_gem_object_pin(obj, vma->vm,
> > > + entry->alignment,
> > > + flags & ~(PIN_GLOBAL | PIN_MAPPABLE));
> > > if (ret)
> > > return ret;
> > >
> > > @@ -605,13 +665,13 @@ eb_vma_misplaced(struct i915_vma *vma)
> > > vma->node.start & (entry->alignment - 1))
> > > return true;
> > >
> > > - if (entry->flags & __EXEC_OBJECT_NEEDS_MAP && !obj->map_and_fenceable)
> > > - return true;
> > > -
> > > if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS &&
> > > vma->node.start < BATCH_OFFSET_BIAS)
> > > return true;
> > >
> > > + if (entry->flags & __EXEC_OBJECT_NEEDS_MAP && !obj->map_and_fenceable)
> > > + return !only_mappable_for_reloc(entry->flags);
> >
> > Hm, this seems to contradict your commit message a bit since it'll result
> > in a behavioural change of what we try to push into mappable for relocs.
> >
> > Shouldn't we instead just clear the NEEDS_MAP flag in reserve_vma when the
> > mappable pin fails and we fall back?
>
> This pair of chunks is required to prevent the ping-pong you just
> described, which was very slow.
Makes sense, but imo then requires a comment (e.g. "prevent costly
ping-pong just for relocations") and some words in the commmit message.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] agp/intel: Serialise after GTT updates
On Tue, Jan 27, 2015 at 09:44:01PM +, Chris Wilson wrote: > On Tue, Jan 27, 2015 at 03:58:05PM +0100, Daniel Vetter wrote: > > On Mon, Jan 26, 2015 at 10:47:10AM +, Chris Wilson wrote: > > > An interesting bug occurs on Pineview through which the root cause is > > > that the writes of the PTE values into the GTT is not serialised with > > > subsequent memory access through the GTT (when using WC updates of the > > > PTE values). This is despite there being a posting read after the GTT > > > update. However, by changing the address of the posting read, the memory > > > access is indeed serialised correctly. > > > > > > Whilst we are manipulating the memory barriers, we can remove the > > > compiler :memory restraint on the intermediate PTE writes knowing that > > > we explicitly perform a posting read afterwards. > > > > > > v2: Replace posting reads with explicit write memory barriers - in > > > particular this is advantages in case of single page objects. Update > > > comments to mention this issue is only with WC writes. > > > > > > Testcase: igt/gem_exec_big #pnv > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88191 > > > Tested-by: huax...@intel.com (v1) > > > Signed-off-by: Chris Wilson > > > Cc: Daniel Vetter > > > > Reviewed-by: Daniel Vetter > > > > Shouldn't we Cc: sta...@vger.kernel.org too? > > Yes, if we can narrow down a user bug it fixes, definitely. Right makes sense, so meanwhile queued for next only. -Daneil -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 05/12] drm/i915/dsi: remove unnecessary dsi device callbacks
On Wed, Jan 28, 2015 at 10:38:45AM +0530, Shobhit Kumar wrote: > On 01/27/2015 06:43 PM, Chris Wilson wrote: > >On Tue, Jan 27, 2015 at 02:09:21PM +0100, Daniel Vetter wrote: > >>On Tue, Jan 27, 2015 at 02:11:18PM +0530, Shobhit Kumar wrote: > >>>On 01/23/2015 08:52 PM, Daniel Vetter wrote: > On Fri, Jan 23, 2015 at 03:14:41PM +0530, Shobhit Kumar wrote: > >On 01/22/2015 06:53 PM, Jani Nikula wrote: > >>On Thu, 22 Jan 2015, Shobhit Kumar > >>wrote: > >>>There had been a instance where we had to drive different resolution > >>>(lower) than the native one. Also in VBT there is a field to make this > >>>generic at least from driver perspective to give the needed target > >>>resolution. In case target resolution is same as native, nothing gets > >>>changed, else mode_fixup function adjusts the mode accordingly keeping > >>>timing as same and enabling scalar. Might not be useful in general, but > >>>did find a use internally. > >> > >>Can we just have the driver return the desired mode from .get_modes in > >>that case? > > > >Okay, I think I did not explain correctly. Get modes is modified to give > >the > >needed target mode only so that userspace creates buffer of the needed > >resolution, but in fixup which is called at modeset, we correct the > >adjusted_mode back to have native resolutions so that modeset is > >correctly > >done. if we do not do like this, during modeset resolutions will be > >wrong as > >per the timings. > > I'm confused. Can you please give an example in real numbers about the > different resolution and how it's all fixed up in hw? > > E.g. 800x600 framebuffer -> pfit -> 1024x756 panel, > > get_modes gives 800x600, adjusted mode corrects to 1024x756. And please > >>> > >>>We had a 19x12 DSI panel which we needed to drive at 12x8 due to lack of > >>>12x8 panels for testing purposes. So get_modes returned 12x8 so that user > >>>space gave 12x8 FBs, and internally in mode_fixup we adjusted correctly for > >>>the 19x12 panel timings and enabled pfit > >> > >>Hm, is that a real use-case shipping to customers or just a hack for > >>development? In the later case I think we can just hardcode the edid for > >>edp ... > > > >Also how is this different from userspace creating a 800x600 mode and > >giving it to the kernel which then uses the pfitter to display it at > >native resolution. That is how it works today. This should also be > >possible with a video= parameter... > > Its different in a way, that user space changes will need a new system build > which is not allowed as per the requirements that we had and hence no hard > coding in code anywhere as well. > > As I said earlier also that this case might not be useful in general and I > am okay to remove this callback. Yeah I think if rebuilding vbt for testing is ok, but rebuilding the kernel isn't then that just smells like needless red tape. I'll reconsider as soon as we need this for shipping systems of course. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support
On Mon, Jan 26, 2015 at 11:08:43AM +, Tvrtko Ursulin wrote: > > On 01/24/2015 09:47 AM, Daniel Vetter wrote: > >On Fri, Jan 23, 2015 at 5:49 PM, Tvrtko Ursulin > > wrote: > >>Could you please translate this into something understandable by newcomers? > >>:) > > > >I don't know which parts are confusing without questions so please ask > >them ... the questions below about scheduler interactions seem fairly > >advanced ;-) > > I suppose for starters, when I took over this work I read the last thread > from December. There you were saying to Jesse to convert the separate ioctl > into execbuf interface for in & out fences. > > That's what I did I thought. So is that not the fashion of the day any more > or I misunderstood something? > > People will scream very soon for something along these lines anyway since it > is kind of packaged with the scheduler I am told and both will be very much > desired soon. That's still the proper fashion imo and what I've thought I've explained ... About the screaming: The real blocker here is destaging android fences. I've been telling this to everyone for a while already, but can't hurt to reinforce ;-) > Idea is to allow submitting of work and not block in userspace rather let > the scheduler queue and shuffle depending on fences. Well that's also possible without explicit fences, but not if your userspace assumes explicit fences exist ofc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Implement WaForceContextSaveRestoreNonCoherent
On Tue, 27 Jan 2015, Damien Lespiau wrote:
Missing commit message. I need some description to decide whether this
is required for fixes/stable or not.
BR,
Jani.
> Signed-off-by: Damien Lespiau
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ae8ea42..47bc4e2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5253,6 +5253,7 @@ enum punit_power_well {
>
> /* GEN8 chicken */
> #define HDC_CHICKEN0 0x7300
> +#define HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT (1<<5)
> #define HDC_FORCE_NON_COHERENT (1<<4)
> #define HDC_DONOT_FETCH_MEM_WHEN_MASKED (1<<11)
> #define HDC_FENCE_DEST_SLM_DISABLE (1<<14)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 36dad33..d393026 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -790,9 +790,11 @@ static int bdw_init_workarounds(struct intel_engine_cs
> *ring)
>*/
> /* WaForceEnableNonCoherent:bdw */
> /* WaHdcDisableFetchWhenMasked:bdw */
> + /* WaForceContextSaveRestoreNonCoherent:bdw */
> /* WaDisableFenceDestinationToSLM:bdw (GT3 pre-production) */
> WA_SET_BIT_MASKED(HDC_CHICKEN0,
> HDC_FORCE_NON_COHERENT |
> + HDC_FORCE_CONTEXT_SAVE_RESTORE_NON_COHERENT |
> HDC_DONOT_FETCH_MEM_WHEN_MASKED |
> (IS_BDW_GT3(dev) ? HDC_FENCE_DEST_SLM_DISABLE : 0));
>
> --
> 1.8.3.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support
On Mon, Jan 26, 2015 at 09:08:03AM +, Chris Wilson wrote: > On Mon, Jan 26, 2015 at 08:52:39AM +0100, Daniel Vetter wrote: > > I think the problem will be platforms that want full explicit fence (like > > android) but allow delayed creation of the fence fd from a gl sync object > > (like the android egl extension allows). > > > > I'm not sure yet how to best expose that really since just creating a > > fence from the implicit request attached to the batch might upset the > > interface purists with the mix in implicit and explicit fencing ;-) Hence > > why I think for now we should just do the eager fd creation at execbuf > > until ppl scream (well maybe not merge this patch until ppl scream ...). > > Everything we do is buffer centric. Even in the future with random bits > of memory, we will still use buffers behind the scenes. From an > interface perspective, it is clearer to me if we say "give me a fence for > this buffer". Exactly the same way as we say "is this buffer busy" or > "wait on this buffer". The change is that we now hand back an fd to slot > into an event loop. That, to me, is a much smaller evolutionary step of > the existing API, and yet more versatile than just attaching one to the > execbuf. The problem is that big parts of the world do not subscribe to that buffer centric view of gfx. Imo since those parts will be the primary users of this interface we should try to fit their ideas as much as feasible. Later on (if we need it) we can add some glue to tie in the buffer-centric implicit model with the explicit model. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support
On Wed, Jan 28, 2015 at 10:22:15AM +0100, Daniel Vetter wrote: > On Mon, Jan 26, 2015 at 09:08:03AM +, Chris Wilson wrote: > > On Mon, Jan 26, 2015 at 08:52:39AM +0100, Daniel Vetter wrote: > > > I think the problem will be platforms that want full explicit fence (like > > > android) but allow delayed creation of the fence fd from a gl sync object > > > (like the android egl extension allows). > > > > > > I'm not sure yet how to best expose that really since just creating a > > > fence from the implicit request attached to the batch might upset the > > > interface purists with the mix in implicit and explicit fencing ;-) Hence > > > why I think for now we should just do the eager fd creation at execbuf > > > until ppl scream (well maybe not merge this patch until ppl scream ...). > > > > Everything we do is buffer centric. Even in the future with random bits > > of memory, we will still use buffers behind the scenes. From an > > interface perspective, it is clearer to me if we say "give me a fence for > > this buffer". Exactly the same way as we say "is this buffer busy" or > > "wait on this buffer". The change is that we now hand back an fd to slot > > into an event loop. That, to me, is a much smaller evolutionary step of > > the existing API, and yet more versatile than just attaching one to the > > execbuf. > > The problem is that big parts of the world do not subscribe to that buffer > centric view of gfx. Imo since those parts will be the primary users of > this interface we should try to fit their ideas as much as feasible. Later > on (if we need it) we can add some glue to tie in the buffer-centric > implicit model with the explicit model. They won't be using execbuffer either. So what's your point? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3] drm/i915: Android native sync support
On Tue, Jan 27, 2015 at 01:43:02PM +, Tvrtko Ursulin wrote: > > On 01/27/2015 12:18 PM, Chris Wilson wrote: > >On Tue, Jan 27, 2015 at 12:13:14PM +, Tvrtko Ursulin wrote: > >>On 01/27/2015 11:40 AM, Chris Wilson wrote: > > [snip] > > >>>Explain how fence_release interacts with enable_signalling. Presumably > >>>either the core fence routines cleanup the outstanding signalling, or we > >>>are expected to. Doing so will remove the requirement for the extra > >>>ref/unref (including the worker). > >> > >>I think normally we would be expected to use fence_get/put on the > >>signaling side of things but that is not possible here since struct > >>fence is embedded in the request. > >> > >>So as it is, sync_fence_release will tear everything down with our > >>callback still on the irq_queue which is what taking a request > >>reference sorts out. > > > >So you are saying that we have a callback for when the fence is > >discarded by userspace and we ignore that opportunity for disabling > >interrupt generation, and remove the extra request references? > > I can change it to work like that sure, don't see anything obvious > preventing this rework. Can use the fence lock to avoid wait queue removal > race and that should be pretty much it. > > On a related not, do you have any ideas for submitting a batch which can > either keep the GPU busy for a configurable time, or until signaled by > something else (possibly another batch). Gen agnostic ideally? :) self-tuning busy loads using the blitter/rendercopy. We have bazillion of existing copypasta in igt tests for this, would be great if someone extracts it into a real igt helper library and converts everyone over. Iirc the one in kms_flip or gem_wait is probably the best starting point. There's no way afaik to block a batch on an mbox or similar from userspace. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3] drm/i915: Android native sync support
On Tue, Jan 27, 2015 at 11:29:36AM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> Add Android native sync support with fences exported as file descriptors via
> the execbuf ioctl (rsvd2 field is used).
>
> This is a continuation of Jesse Barnes's previous work, squashed to arrive at
> the final destination, cleaned up, with some fixes and preliminary light
> testing.
>
> GEM requests are extended with fence structures which are associated with
> Android sync fences exported to user space via file descriptors. Fences which
> are waited upon, and while exported to userspace, are referenced and added to
> the irq_queue so they are signalled when requests are completed. There is no
> overhead apart from the where fences are not requested.
>
> Based on patches by Jesse Barnes:
>drm/i915: Android sync points for i915 v3
>drm/i915: add fences to the request struct
>drm/i915: sync fence fixes/updates
>
> To do:
>* Extend driver data with context id / ring id (TBD).
>
> v2:
>* Code review comments. (Chris Wilson)
>* ring->add_request() was a wrong thing to call - rebase on top of John
> Harrison's (drm/i915: Early alloc request) to ensure correct request is
> present before creating a fence.
>* Take a request reference from signalling path as well to ensure request
> sticks around while fence is on the request completion wait queue.
>
> v3:
>* Use worker to unreference on completion so it can lock the mutex from
> interrupt context.
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Jesse Barnes
> Cc: John Harrison
btw for merging we need to split the conversion to fences out from the
actual userspace interface. And imo we should replace a lot of our
existing usage of i915_gem_request with fences within the driver, too. Not
tacked on the side like in your patch here. All the recent refactoring
have been aiming to match i915 requests to the internal fence interfaces,
so we should be pretty much there.
We also need this to be able to integrate with the scheduler properly - if
that needs to deal both with fences and i915 requests separately it'll be
a bit more messy. If it's all just fences the code should be streamlined a
lot.
Sorry for spotting this only just now, I've thought Jesse's old patches
had this already and you've carried that approach over.
-Daniel
> ---
> drivers/gpu/drm/i915/Kconfig | 14 ++
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.h| 27 +++
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 24 +++
> drivers/gpu/drm/i915/i915_sync.c | 254
> +
> include/uapi/drm/i915_drm.h| 8 +-
> 6 files changed, 325 insertions(+), 3 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/i915_sync.c
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index 4e39ab3..abafe68 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -43,6 +43,20 @@ config DRM_I915_KMS
>
> If in doubt, say "Y".
>
> +config DRM_I915_SYNC
> + bool "Enable explicit sync support"
> + depends on DRM_I915
> + default y if STAGING
> + select STAGING
> + select ANDROID
> + select SYNC
> + help
> + Choose this option to enable Android native sync support and the
> + corresponding i915 driver code to expose it. Slightly increases
> + driver size and pulls in sync support from staging.
> +
> + If in doubt, say "Y".
> +
> config DRM_I915_FBDEV
> bool "Enable legacy fbdev support for the modesetting intel driver"
> depends on DRM_I915
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 16e3dc3..bcff481 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -58,6 +58,7 @@ i915-y += intel_audio.o \
> intel_sprite.o
> i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o
> i915-$(CONFIG_DRM_I915_FBDEV)+= intel_fbdev.o
> +i915-$(CONFIG_DRM_I915_SYNC) += i915_sync.o
>
> # modesetting output/encoder code
> i915-y += dvo_ch7017.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 023f422..c4d254c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -49,6 +49,7 @@
> #include
> #include
> #include
> +#include
>
> /* General customization:
> */
> @@ -2092,6 +2093,15 @@ struct drm_i915_gem_request {
> struct list_head client_list;
>
> uint32_t uniq;
> +
> +#ifdef CONFIG_DRM_I915_SYNC
> + /* core fence obj for this request, may be exported */
> + struct fence fence;
> +
> + wait_queue_t wait;
> +
> + struct work_struct unref_work;
> +#endif
> };
>
> void i915_gem_request_free(struct kref *req_ref);
> @@ -2583,6 +2593,23 @@ void i915_init_vm(struct drm_i915_private *dev_priv,
> void i915_gem_free_object(struct drm_gem_obj
Re: [Intel-gfx] [PATCH] drm/i915: Drop pipe_enable checks in vblank funcs
On Mon, Jan 26, 2015 at 11:03:14PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
>
> Thank you for the patch.
>
> On Monday 26 January 2015 07:41:59 Daniel Vetter wrote:
> > With Ville's rework to use drm_crtc_vblank_on/off the core will take
> > care of rejecting drm_vblank_get calls when the pipe is off.
>
> After debugging a related issue in the omapdrm driver I think this is only
> partially true. drm_vblank_off() will increase the vblank refcount, resulting
> in drm_vblank_get() returning an error instead of calling the driver's
> .enable_vblank operation. However, this only works if drm_vblank_off() has
> been called, not when the pipe is off because it hasn't been turned on yet.
We recover this state at driver load by manually calling off/on. See
intel_sanitize_crtc
> Consider this case. The driver has just been loaded, all pipes are off. An
> application opens the DRM device node and closes it without enabling the
> pipe.
> The lastclose operation will be called, and it will call
> drm_fb_helper_restore_fbdev_mode_unlocked() to restore the CRTC. This
> triggers
> the timeout warning in drm_wait_one_vblank with omapdrm when using the atomic
> update transitional helpers. The callstack is as follows.
>
> [] (drm_wait_one_vblank [drm])
> [] (drm_plane_helper_commit [drm_kms_helper])
> [] (drm_crtc_helper_set_mode [drm_kms_helper])
> [] (drm_crtc_helper_set_config [drm_kms_helper])
> [] (drm_mode_set_config_internal [drm])
> [] (restore_fbdev_mode [drm_kms_helper])
> [] (drm_fb_helper_restore_fbdev_mode_unlocked [drm_kms_helper])
> [] (dev_lastclose [omapdrm])
> [] (drm_lastclose [drm])
> [] (drm_release [drm])
> [] (__fput)
> [] (task_work_run)
> [] (do_work_pending)
>
> The drm_crtc_vblank_get() call in drm_plane_helper_commit() will not return
> an
> error as drm_vblank_off() has never been called so far.
>
> The exact same issue obviously doesn't affect the i915 driver as it doesn't
> use the transitional helpers, but a different issue could be present with the
> same root cause.
We use the transitional helpers, code will land in drm-next soonish.
> Where do you think this should be fixed ?
Proper hw state readout for smooth booting is hard. I've tried to aim a
bit in that direction with the ->reset callbacks for atomic, but maybe we
should do a new helper inspired by i915 just for this kind of stuff ...
My idea would be to add ->read_hw_state hooks which will out the various
drm_*_state structs and then use that for both ->reset and for
cross-checking after modesets. And then we could shovel a bunch of the
common code to sanitize hw state into the shared ->reset functions.
But given that no one else but i915 tries really hard to reconstruct hw
state probably not yet worth it.
-Daniel
>
> > Also the core won't call the get_vblank_counter hooks in that case either.
> > And since we've dropped ums support recently we can now remove these
> > hacks, yay!
> >
> > Noticed while trying to answer questions Laurent had about how the new
> > atomic helpers work.
> >
> > Cc: Laurent Pinchart
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 52 --
> > 1 file changed, 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c index 2399eaed2ca3..4761f7fe6fb1 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -492,31 +492,6 @@ static void i915_enable_asle_pipestat(struct drm_device
> > *dev) spin_unlock_irq(&dev_priv->irq_lock);
> > }
> >
> > -/**
> > - * i915_pipe_enabled - check if a pipe is enabled
> > - * @dev: DRM device
> > - * @pipe: pipe to check
> > - *
> > - * Reading certain registers when the pipe is disabled can hang the chip.
> > - * Use this routine to make sure the PLL is running and the pipe is active
> > - * before reading such registers if unsure.
> > - */
> > -static int
> > -i915_pipe_enabled(struct drm_device *dev, int pipe)
> > -{
> > - struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > - if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > - /* Locking is horribly broken here, but whatever. */
> > - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > -
> > - return intel_crtc->active;
> > - } else {
> > - return I915_READ(PIPECONF(pipe)) & PIPECONF_ENABLE;
> > - }
> > -}
> > -
> > /*
> > * This timing diagram depicts the video signal in and
> > * around the vertical blanking period.
> > @@ -583,12 +558,6 @@ static u32 i915_get_vblank_counter(struct drm_device
> > *dev, int pipe) unsigned long low_frame;
> > u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
> >
> > - if (!i915_pipe_enabled(dev, pipe)) {
> > - DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
> > - "pipe %c\n", pipe_na
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Fallback to using CPU relocations for large batch buffers
On Wed, Jan 28, 2015 at 10:14:47AM +0100, Daniel Vetter wrote:
> On Tue, Jan 27, 2015 at 09:43:00PM +, Chris Wilson wrote:
> > On Tue, Jan 27, 2015 at 04:09:37PM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 14, 2015 at 11:20:56AM +, Chris Wilson wrote:
> > > > static int
> > > > i915_gem_execbuffer_reserve_vma(struct i915_vma *vma,
> > > > struct intel_engine_cs *ring,
> > > > @@ -536,14 +589,21 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma
> > > > *vma,
> > > > int ret;
> > > >
> > > > flags = 0;
> > > > - if (entry->flags & __EXEC_OBJECT_NEEDS_MAP)
> > > > - flags |= PIN_GLOBAL | PIN_MAPPABLE;
> > > > - if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
> > > > - flags |= PIN_GLOBAL;
> > > > - if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS)
> > > > - flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> > > > + if (!drm_mm_node_allocated(&vma->node)) {
> > > > + if (entry->flags & __EXEC_OBJECT_NEEDS_MAP)
> > > > + flags |= PIN_GLOBAL | PIN_MAPPABLE;
> > > > + if (entry->flags & EXEC_OBJECT_NEEDS_GTT)
> > > > + flags |= PIN_GLOBAL;
> > > > + if (entry->flags & __EXEC_OBJECT_NEEDS_BIAS)
> > > > + flags |= BATCH_OFFSET_BIAS | PIN_OFFSET_BIAS;
> > > > + }
> > >
> > > Hm, aren't we always calling reserve un buffers we know we've just
> > > unbound? Keeping the flags computation would at least be a good selfcheck
> > > about the consistency of our placing decisions, so I'd like to keep it.
>
> I still think this isn't required, even with the ping-pong preventer below
> kept. Maybe add a WARN_ON(drm_mm_node_allocated); just for paranoia
> instead?
The issue I have with this chunk here is that we have two not quite
duplicate pieces of code deciding when to migrate an object:
i915_vma_misplaced() vs eb_vma_misplaced().
Maybe a __i915_vma_pin(), but as it stands atm
i915_gem_object_pin_view() requires some TLC first.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Remove nested work in gpu error handling
On Tue, Jan 27, 2015 at 09:53:20AM +, Chris Wilson wrote: > On Mon, Jan 26, 2015 at 06:03:05PM +0200, Mika Kuoppala wrote: > > Now when we declare gpu errors only through our own dedicated > > hangcheck workqueue there is no need to have a separate workqueue > > for handling the resetting and waking up the clients as the deadlock > > concerns are no more. > > > > The only exception is i915_debugfs::i915_set_wedged, which triggers > > error handling through process context. However as this is only used through > > test harness it is responsibility for test harness not to introduce hangs > > through both debug interface and through hangcheck mechanism at the same > > time. > > > > Remove gpu_error.work and let the hangcheck work do the tasks it used to. > > > > Signed-off-by: Mika Kuoppala > > For our own sanity, we need to stick some form of that comment in > i915_set_wedged(), so that when we do inevitably blow up, we can laugh > at ourselves. Yeah that has the potential for some self-inflicted pain. I've merged all the other patches meanwhile, thanks. -Daniel > > Otherwise, lgtm. > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in > this WARN at boot (and pretty frequently afterwards): > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > gen6_set_rps+0x371/0x3c0() > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > Modules linked in: > CPU: 0 PID: 989 Comm: kworker/0:2 Not tainted 3.19.0-rc6 #31 > Hardware name: LENOVO 20A7002WUS/20A7002WUS, BIOS GRET38WW (1.15 ) 05/29/2014 > Workqueue: events intel_gen6_powersave_work > 81a82dd0 817f099e 88021451bd28 > 8107d107 8802148e 0022 8802148e86f0 > 88021498f000 0004 8107d185 81a83448 > Call Trace: > [] ? dump_stack+0x40/0x50 > [] ? warn_slowpath_common+0x77/0xb0 > [] ? warn_slowpath_fmt+0x45/0x50 > [] ? gen6_set_rps+0x371/0x3c0 > [] ? intel_gen6_powersave_work+0x780/0x1180 > [] ? process_one_work+0x130/0x350 > [] ? worker_thread+0x114/0x450 > [] ? rescuer_thread+0x2f0/0x2f0 > [] ? kthread+0xbc/0xe0 > [] ? kthread_create_on_node+0x170/0x170 > [] ? ret_from_fork+0x7c/0xb0 > [] ? kthread_create_on_node+0x170/0x170 > ---[ end trace c3ac159c87b9b234 ]--- > > I bisected this back to: > > commit 93ee29203f506582cca2bcec5f05041526d9ab0a > Author: Tom O'Rourke > Date: Wed Nov 19 14:21:52 2014 -0800 > > drm/i915: Use efficient frequency for HSW/BDW > > Added gen6_init_rps_frequencies() to initialize > the rps frequency values. This function replaces > parse_rp_state_cap(). In addition to reading RPn, > RP0, and RP1 from RP_STATE_CAP register, the new > function reads efficient frequency (aka RPe) from > pcode for Haswell and Broadwell and sets the turbo > softlimits. The turbo minimum frequency softlimit > is set to RPe for Haswell and Broadwell and to RPn > otherwise. > > For RPe, the efficiency is based on the frequency/power > ratio (MHz/W); this is considering GT power and not > package power. The efficent frequency is the highest > frequency for which the frequency/power ratio is within > some threshold of the highest frequency/power ratio. > A fixed decrease in frequency results in smaller > decrease in power at frequencies less than RPe than > at frequencies above RPe. > > v2: Following suggestions from Chris Wilson and > Daniel Vetter to extend and rename parse_rp_state_cap > and to open-code a poorly named function. > > Signed-off-by: Tom O'Rourke > Reviewed-by: Chris Wilson > [danvet: Remove unused variables.] > Signed-off-by: Daniel Vetter > > I'm not at all familiar with this hardware, but I took a quick look into > what changed with this commit for my laptop. Before the commit, > rps.min_freq_softlimit is 4 (from rps.min_freq) and > rps.max_freq_softlimit is 22. > > After the commit, rps.min_freq_softlimit is set to the > rps.efficient_freq value read from pcode, which is 34 on my laptop. > So later when gen6_set_rps() is called with rps.min_freq_softlimit that > warning is hit. > > Any thoughts? It certainly seems fishy that this commit causes > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. Sounds like the rpe value on your firmware is rather bogus, and we probably need to sanity-check it. Tom? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
On Tue, 27 Jan 2015, Martin Wilck wrote:
> Dear list members,
>
> I suffer from highly disturbing artefacts with the Intel HD 4400 chip
> set on thw 2560x1440 display (Sharp LQ133T1JW19) of my Fujitsu Lifebook
> S904. The effects are hard to describe, therefore I have uploaded a
> video to
> https://www.dropbox.com/sh/9wh0nzvp0w1phxz/AAAsvAqHN8ApXzor58FmGbwLa?dl=0.
> I can distinguish 2 effects that are possibly related to each other:
Just a quick guess, what does this say:
# cat /sys/module/i915/parameters/enable_{fbc,ips,psr}
Please try these module parameters for the non-zero ones:
i915.enable_fbc=0
i915.enable_ips=0
i915.enable_psr=0
BR,
Jani.
>
> 1. Maximized windows under GNOME3 (fully maximized or half-maximized on
> the right edge of the screen) fill the screen vertically with a single
> color (usually grey). Half-maximzed windows on the left edge are not
> affected. The effect is only seen if the Laptop screen is the main
> monitor in GNOME (showing the panel on top). Some applications, which
> start in maximized mode and can't be resized, are unusable on this screen.
>
> 2. Artefacts appear when windows are placed on the right and bottom
> areas of the screen. The artefacts look as if pixels extend vertically
> from their horizontal position to the bottom edge of the screen
> (resulting in vertical lines). The strength of the effect depends on the
> content of the windows that are visible. The effect "extends" from the
> window causing it to other windows and the background. The effect is not
> seen in screenshots (screenshot looks fine although the screen was
> unreadable at the time I took it).
>
> 1.) is observed only under GNOME, but I see 2.) on KDE and LXDE, too. I
> also tried various distributions. Most of my experiments have been done
> on OpenSUSE 13.2, kernel-vanilla-3.19.rc5, xorg 7.6_1.16.1-5.1, intel
> 2.99.916, libdrm_intel1-2.4.58-1.1, Mesa 10.3.0-91.3.2. But I see the
> artefacts also on Fedora 21 and Ubuntu 14.04.
>
> As a matter of fact, I tried the modesetting driver and even fbdev and
> observed similar effects, too. Thus i915 may actually not be the culprit
> here, but could I figure no other place to ask for help than this
> mailing list.
>
> I have also experimented with resolutions and refresh rates. Only at
> 1024x768 and lower I was unable to reproduce the problem.
>
> Any help would be highly appreciated. I am highly willing to do any
> testing and/or debugging that may be necessary to find the problem.
>
> Best regards
> Martin
>
>
> martin@artemis:~> xrandr --verbose
> Screen 0: minimum 8 x 8, current 2560 x 1440, maximum 32767 x 32767
> eDP1 connected primary 2560x1440+0+0 (0x49) normal (normal left inverted
> right x axis y axis) 294mm x 165mm
> Identifier: 0x43
> Timestamp: 19447
> Subpixel: unknown
> Gamma: 1.0:1.0:1.0
> Brightness: 1.0
> Clones:
> CRTC: 0
> CRTCs: 0 1 2
> Transform: 1.00 0.00 0.00
> 0.00 1.00 0.00
> 0.00 0.00 1.00
> filter:
> _MUTTER_PRESENTATION_OUTPUT: 0
> EDID:
> 00004d101214
> 24170104a51d1178068800a4564d9a26
> 0d50540001010101010101010101
> 010101010101735f00a0a0a03a503020
> e50026a51018454c00a0a0a03a50
> 3020e50026a51018aa3f00a0a0a0
> 3a503020e50026a5101800fc
> 004c513154314a5731390a2000b6
> BACKLIGHT: 892
> range: (0, 892)
> Backlight: 892
> range: (0, 892)
> scaling mode: Full aspect
> supported: None, Full, Center, Full aspect
> Broadcast RGB: Automatic
> supported: Automatic, Full, Limited 16:235
> audio: auto
> supported: force-dvi, off, auto, on
> 2560x1440 (0x49) 244.350MHz -HSync -VSync *current +preferred
> h: width 2560 start 2608 end 2640 total 2720 skew0 clock
> 89.83KHz
> v: height 1440 start 1454 end 1459 total 1498 clock
> 59.97Hz
> 2560x1440 (0xae) 195.250MHz -HSync -VSync
> h: width 2560 start 2608 end 2640 total 2720 skew0 clock
> 71.78KHz
> v: height 1440 start 1454 end 1459 total 1498 clock
> 47.92Hz
> 2560x1440 (0xaf) 162.980MHz -HSync -VSync
> h: width 2560 start 2608 end 2640 total 2720 skew0 clock
> 59.92KHz
> v: height 1440 start 1454 end 1459 total 1498 clock
> 40.00Hz
> 1920x1440 (0xb0) 234.000MHz -HSync +VSync
> h: width 1920 start 2048 end 2256 total 2600 skew0 clock
> 90.00KHz
> v: height 1440 start 1441 end 1444 total 1500 clock
> 60.00Hz
> 1856x1392 (0xb1) 218.300MHz -HSync +VSync
> h: width 1856 start 1952 end 2176 total 2528 skew0 clock
> 86.35KHz
> v: height 1392 start 1393 end 1396 total 1439 clock
> 60.01Hz
>
Re: [Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
On Tue, Jan 27, 2015 at 10:49:43PM +0100, Martin Wilck wrote: > Dear list members, > > I suffer from highly disturbing artefacts with the Intel HD 4400 chip > set on thw 2560x1440 display (Sharp LQ133T1JW19) of my Fujitsu Lifebook > S904. The effects are hard to describe, therefore I have uploaded a > video to > https://www.dropbox.com/sh/9wh0nzvp0w1phxz/AAAsvAqHN8ApXzor58FmGbwLa?dl=0. > I can distinguish 2 effects that are possibly related to each other: [snip] > 1.) is observed only under GNOME, but I see 2.) on KDE and LXDE, too. I > also tried various distributions. Most of my experiments have been done > on OpenSUSE 13.2, kernel-vanilla-3.19.rc5, xorg 7.6_1.16.1-5.1, intel > 2.99.916, libdrm_intel1-2.4.58-1.1, Mesa 10.3.0-91.3.2. But I see the > artefacts also on Fedora 21 and Ubuntu 14.04. > > As a matter of fact, I tried the modesetting driver and even fbdev and > observed similar effects, too. Thus i915 may actually not be the culprit > here, but could I figure no other place to ask for help than this > mailing list. From watching the video, and your comments here, it is clear this is not a driver bug (neither in the ddx nor opengl compositor). That leaves us with hardware misconfiguration, aka kernel bug. Some of the artefacts could be a display underrun. It would be good to check a drm.debug=0xe dmesg for any warnings (and the info itself will be useful). Another thing worth playing with is i915.enable_fbc=1 (though that is really just swapping one problem with another). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
intel_uncore_early_sanitize() will reset the forcewake registers. When
forcewake domains were introduced, the domain init was done after the
sanitization of the forcewake registers. And as the resetting of
registers use the domain accessors, we tried to reset the forcewake
registers with unitialized forcewake domains and failed.
Fix this by sanitizing after all the domains have been initialized.
On ivb we need special care as there we need early forcewake access to
determine the final configuration for the forcewake domain.
This regression was introduced in
commit 05a2fb157e44a53c79133805d30eaada43911941
Author: Mika Kuoppala
Date: Mon Jan 19 16:20:43 2015 +0200
drm/i915: Consolidate forcewake code
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
Reported-by: Olof Johansson
Tested-by: Darren Hart
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index b3951f2..c438ca4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private *dev_priv)
static inline void
fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
{
+ WARN_ON(d->reg_set == 0);
__raw_i915_write32(d->i915, d->reg_set, d->val_reset);
}
@@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum
forcewake_domains fw_do
struct intel_uncore_forcewake_domain *d;
enum forcewake_domain_id id;
+ WARN_ON(dev_priv->uncore.fw_domains == 0);
+
for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
fw_domain_reset(d);
@@ -987,8 +990,7 @@ static void fw_domain_init(struct drm_i915_private
*dev_priv,
void intel_uncore_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
-
- __intel_uncore_early_sanitize(dev, false);
+ bool sanitize_done = false;
if (IS_GEN9(dev)) {
dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
@@ -1037,6 +1039,8 @@ void intel_uncore_init(struct drm_device *dev)
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_MT, FORCEWAKE_MT_ACK);
+ __intel_uncore_early_sanitize(dev, false);
+ sanitize_done = true;
mutex_lock(&dev->struct_mutex);
fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
ecobus = __raw_i915_read32(dev_priv, ECOBUS);
@@ -1058,6 +1062,9 @@ void intel_uncore_init(struct drm_device *dev)
FORCEWAKE, FORCEWAKE_ACK);
}
+ if (sanitize_done == false)
+ __intel_uncore_early_sanitize(dev, false);
+
switch (INTEL_INFO(dev)->gen) {
default:
MISSING_CASE(INTEL_INFO(dev)->gen);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tools/intel_reg_read: Adding the reg offset for VLV and CHT
On Wed, Jan 28, 2015 at 02:19:50PM +0530, meghanelogal wrote:
> From: meghanelogal
>
> For VLV and CHT for each register access we need to add base offset of
> 0x18.
>
> Signed-off-by: meghanelogal
Nah, vlv/cht have new decoder tools, and skl will have yet another one.
Unfortunately no one unified them yet really. See tools/quick_dump.
-Daniel
> ---
> tools/intel_reg_read.c | 20 ++--
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
> index 3b91291..c550b02 100644
> --- a/tools/intel_reg_read.c
> +++ b/tools/intel_reg_read.c
> @@ -48,11 +48,13 @@ static void bit_decode(uint32_t reg)
>
> static void dump_range(uint32_t start, uint32_t end)
> {
> - int i;
> -
> + int i, reg = 0;
> + struct pci_device *dev = intel_get_pci_device();
> + if (IS_CHERRYVIEW(dev->device_id) || IS_VALLEYVIEW(dev->device_id))
> + reg = 0x18;
> for (i = start; i < end; i += 4)
> printf("0x%X : 0x%X\n", i,
> -*(volatile uint32_t *)((volatile char*)mmio + i));
> +*(volatile uint32_t *)((volatile char*)mmio + i + reg));
> }
>
> static void usage(char *cmdname)
> @@ -129,11 +131,17 @@ int main(int argc, char** argv)
> sscanf(argv[i], "0x%x", ®);
> dump_range(reg, reg + (dwords * 4));
>
> - if (decode_bits)
> - bit_decode(*(volatile uint32_t *)((volatile
> char*)mmio + reg));
> + if (decode_bits) {
> + struct pci_device *dev = intel_get_pci_device();
> + if (IS_CHERRYVIEW(dev->device_id) ||
> + IS_VALLEYVIEW(dev->device_id)) {
> + reg += 0x18;
> + bit_decode(*(volatile uint32_t *)
> + ((volatile char*)mmio + reg));
> + }
> + }
> }
> }
> -
> intel_register_access_fini();
>
> out:
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support
On Wed, Jan 28, 2015 at 09:23:46AM +, Chris Wilson wrote: > On Wed, Jan 28, 2015 at 10:22:15AM +0100, Daniel Vetter wrote: > > On Mon, Jan 26, 2015 at 09:08:03AM +, Chris Wilson wrote: > > > On Mon, Jan 26, 2015 at 08:52:39AM +0100, Daniel Vetter wrote: > > > > I think the problem will be platforms that want full explicit fence > > > > (like > > > > android) but allow delayed creation of the fence fd from a gl sync > > > > object > > > > (like the android egl extension allows). > > > > > > > > I'm not sure yet how to best expose that really since just creating a > > > > fence from the implicit request attached to the batch might upset the > > > > interface purists with the mix in implicit and explicit fencing ;-) > > > > Hence > > > > why I think for now we should just do the eager fd creation at execbuf > > > > until ppl scream (well maybe not merge this patch until ppl scream ...). > > > > > > Everything we do is buffer centric. Even in the future with random bits > > > of memory, we will still use buffers behind the scenes. From an > > > interface perspective, it is clearer to me if we say "give me a fence for > > > this buffer". Exactly the same way as we say "is this buffer busy" or > > > "wait on this buffer". The change is that we now hand back an fd to slot > > > into an event loop. That, to me, is a much smaller evolutionary step of > > > the existing API, and yet more versatile than just attaching one to the > > > execbuf. > > > > The problem is that big parts of the world do not subscribe to that buffer > > centric view of gfx. Imo since those parts will be the primary users of > > this interface we should try to fit their ideas as much as feasible. Later > > on (if we need it) we can add some glue to tie in the buffer-centric > > implicit model with the explicit model. > > They won't be using execbuffer either. So what's your point? Android very much will user execbuffer. And even the in-flight buffered svm stuff will keep on using execbuf (just without any relocs). And once we indeed can make the case (plus have the hw) for direct userspace submission I think the kernel shouldn't be in the business of creating fence objects: The ring will be fully under control of userspace, and that's the only place we could insert a seqno into. So again the same trust issues. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/11] drm/i915: Remove pinned check from madvise_ioctl
On Mon, Jan 26, 2015 at 04:43:17AM -0800, Rodrigo Vivi wrote:
> From: Chris Wilson
>
> We don't need to incur the overhead of checking whether the object is
> pinned prior to changing its madvise. If the object is pinned, the
> madvise will not take effect until it is unpinned and so we cannot free
> the pages being pointed at by hardware. Marking a pinned object with
> allocated pages as DONTNEED will not trigger any undue warnings. The check
> is therefore superfluous, and by removing it we can remove a linear walk
> over all the vma the object has.
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Rodrigo Vivi
I'd still would like to see some igt testcase which marks a pinned scanout
buffer asl DONTNEED. Just to make sure we don't accidentally create a gap
somewhere for a CVE to sneak through.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6c40365..123ce34 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4365,11 +4365,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void
> *data,
> goto unlock;
> }
>
> - if (i915_gem_obj_is_pinned(obj)) {
> - ret = -EINVAL;
> - goto out;
> - }
> -
> if (obj->pages &&
> obj->tiling_mode != I915_TILING_NONE &&
> dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
> @@ -4388,7 +4383,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void
> *data,
>
> args->retained = obj->madv != __I915_MADV_PURGED;
>
> -out:
> drm_gem_object_unreference(&obj->base);
> unlock:
> mutex_unlock(&dev->struct_mutex);
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/11] Revert "drm/i915: Fix mutex->owner inspection race under DEBUG_MUTEXES"
On Mon, Jan 26, 2015 at 04:43:22AM -0800, Rodrigo Vivi wrote: > From: Chris Wilson > > The core fix was applied in > > commit a63b03e2d2477586440741677ecac45bcf28d7b1 > Author: Chris Wilson > Date: Tue Jan 6 10:29:35 2015 + > > mutex: Always clear owner field upon mutex_unlock() > > (note the absence of stable@ tag) > > so we can now revert our band-aid commit 226e5ae9e5f910 for -next. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Jani Nikula > Signed-off-by: Rodrigo Vivi Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 0a074bc..b50a2b4 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -5213,7 +5213,7 @@ static bool mutex_is_locked_by(struct mutex *mutex, > struct task_struct *task) > if (!mutex_is_locked(mutex)) > return false; > > -#if defined(CONFIG_SMP) && !defined(CONFIG_DEBUG_MUTEXES) > +#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_MUTEXES) > return mutex->owner == task; > #else > /* Since UP may be pre-empted, we cannot assume that we own the lock */ > -- > 1.9.3 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/11] drm/i915: add irq_barrier operation for synchronising reads
On Mon, Jan 26, 2015 at 04:43:24AM -0800, Rodrigo Vivi wrote:
> From: Dave Gordon
>
> On some generations of chips, it is necessary to read an MMIO register
> before getting the sequence number from the status page in main memory,
> in order to ensure coherency; and on all generations this should be
> either helpful or harmless.
>
> In general, we want this operation to be the cheapest possible, since
> we require only the side-effect of DMA completion and don't interpret
> the result of the read, and don't require any coordination with other
> threads, power domains, or anything else.
>
> However, finding a suitable register may be problematic; on GEN6 chips
> the ACTHD register was used, but on VLV et al access to this register
> requires FORCEWAKE and therefore many complications involving spinlocks
> and polling.
>
> So this commit introduces this synchronising operation as a distinct
> vfunc in the engine structure, so that it can be GEN- or chip-specific
> if needed.
>
> And there are three implementations; a dummy one, for chips where no
> synchronising read is needed, a gen6(+) version that issues a posting
> read (to TAIL), and a VLV-specific one that issues a raw read instead,
> avoiding touching FORCEWAKE and GTFIFO and other such complications.
>
> We then change gen6_ring_get_seqno() to use this new irq_barrier rather
> than a POSTING_READ of ACTHD. Note that both older (pre-GEN6) and newer
> (GEN8+) devices running in LRC mode do not currently include any posting
> read in their own get_seqno() implementations, so this change only
> makes a difference on VLV (and not CHV+).
>
> Signed-off-by: Dave Gordon
> Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> shuang...@intel.com)
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 37
> +++--
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 23020d6..97473ed 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1227,6 +1227,28 @@ pc_render_add_request(struct intel_engine_cs *ring)
> return 0;
> }
>
> +static void
> +dummy_irq_barrier(struct intel_engine_cs *ring)
> +{
> +}
> +
> +static void
> +gen6_irq_barrier(struct intel_engine_cs *ring)
> +{
> + struct drm_i915_private *dev_priv = to_i915(ring->dev);
> + POSTING_READ(RING_TAIL(ring->mmio_base));
> +}
> +
> +#define __raw_i915_read32(dev_priv__, reg__) readl((dev_priv__)->regs +
> (reg__))
> +#define RAW_POSTING_READ(reg__)
> (void)__raw_i915_read32(dev_priv, reg__)
> +
> +static void
> +vlv_irq_barrier(struct intel_engine_cs *ring)
> +{
> + struct drm_i915_private *dev_priv = to_i915(ring->dev);
> + RAW_POSTING_READ(RING_TAIL(ring->mmio_base));
> +}
> +
> static u32
> gen6_ring_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency)
> {
> @@ -1234,8 +1256,7 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring, bool
> lazy_coherency)
>* ivb (and maybe also on snb) by reading from a CS register (like
>* ACTHD) before reading the status page. */
> if (!lazy_coherency) {
> - struct drm_i915_private *dev_priv = ring->dev->dev_private;
> - POSTING_READ(RING_ACTHD(ring->mmio_base));
> + ring->irq_barrier(ring);
> }
Imo just do a vlv_ring_get_seqno if this is a problem. Adding a vfunc with
mostly empty or same implemenation to another very tiny vfunc isn't doing
a whole lot of good to the codebase.
-Daniel
>
> return intel_read_status_page(ring, I915_GEM_HWS_INDEX);
> @@ -2393,6 +2414,7 @@ int intel_init_render_ring_buffer(struct drm_device
> *dev)
> ring->irq_get = gen8_ring_get_irq;
> ring->irq_put = gen8_ring_put_irq;
> ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
> + ring->irq_barrier = gen6_irq_barrier;
> ring->get_seqno = gen6_ring_get_seqno;
> ring->set_seqno = ring_set_seqno;
> if (i915_semaphore_is_enabled(dev)) {
> @@ -2409,6 +2431,10 @@ int intel_init_render_ring_buffer(struct drm_device
> *dev)
> ring->irq_get = gen6_ring_get_irq;
> ring->irq_put = gen6_ring_put_irq;
> ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
> + if (IS_VALLEYVIEW(dev) && !IS_GEN8(dev))
> + ring->irq_barrier = vlv_irq_barrier;
> + else
> + ring->irq_barrier = gen6_irq_barrier;
> ring->get_seqno = gen6_ring_get_seqno;
> ring->set_seqno = ring_set_seqno;
> if (i915_semaphore_is_enabled(dev)) {
> @@ -2435,6 +2461,7 @@ int intel_init_render_ring_buffer(struct drm_device
> *dev)
> } else if (IS_GEN5(dev)) {
> ring->add_request =
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
> Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
> this WARN at boot (and pretty frequently afterwards):
>
> WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377
> gen6_set_rps+0x371/0x3c0()
> WARN_ON(val > dev_priv->rps.max_freq_softlimit)
[snip]
> I'm not at all familiar with this hardware, but I took a quick look into
> what changed with this commit for my laptop. Before the commit,
> rps.min_freq_softlimit is 4 (from rps.min_freq) and
> rps.max_freq_softlimit is 22.
>
> After the commit, rps.min_freq_softlimit is set to the
> rps.efficient_freq value read from pcode, which is 34 on my laptop.
> So later when gen6_set_rps() is called with rps.min_freq_softlimit that
> warning is hit.
>
> Any thoughts? It certainly seems fishy that this commit causes
> rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
Very fishy indeed. Moral of this story, never trust hw.
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e630feb18e4..bbedd2901c54 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct drm_device
*dev)
&ddcc_status);
if (0 == ret)
dev_priv->rps.efficient_freq =
- (ddcc_status >> 8) & 0xff;
+ clamp_t(u8,
+ (ddcc_status >> 8) & 0xff,
+ dev_priv->rps.min_freq,
+ dev_priv->rps.max_freq);
}
/* Preserve min/max settings in case of re-init */
But really it is probably just best to disable the query for hsw:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e630feb18e4..01bd508e81f6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct drm_device
*dev)
dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
+ if (IS_BROADWELL(dev)) {
ret = sandybridge_pcode_read(dev_priv,
HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
&ddcc_status);
Paranoia says we do both.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/11] drm/i915: Extend GET_APERTURE ioctl to report available map space
On Mon, Jan 26, 2015 at 04:43:18AM -0800, Rodrigo Vivi wrote:
> When constructing a batchbuffer, it is sometimes crucial to know the
> largest hole into which we can fit a fenceable buffer (for example when
> handling very large objects on gen2 and gen3). This depends on the
> fragmentation of pinned buffers inside the aperture, a question only the
> kernel can easily answer.
>
> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> include a couple of new fields in its reply to userspace - the total
> amount of space available in the mappable region of the aperture and
> also the single largest block available.
>
> This is not quite what userspace wants to answer the question of whether
> this batch will fit as fences are also required to meet severe alignment
> constraints within the batch. For this purpose, a third conservative
> estimate of largest fence available is also provided. For when userspace
> needs more than one batch, we also provide the culmulative space
> available for fences such that it has some additional guidance to how
> much space it could allocate to fences. Conservatism still wins.
>
> The patch also adds a debugfs file for convenient testing and reporting.
>
> v2: The first object cannot end at offset 0, so we can use last==0 to
> detect the empty list.
>
> v3: Expand all values to 64bit, just in case.
> Report total mappable aperture size for userspace that cannot easily
> determine it by inspecting the PCI device.
>
> v4: (Rodrigo) Fixed rebase conflicts.
>
> Signed-off-by: Chris Wilson
> Signed-off-by: Rodrigo Vivi
Do we have the libdrm patch for this too? Imo there's not much use in this
if mesa remains broken, especially since this is for gen2/3 ... most DE
use gl nowadays.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 27 +
> drivers/gpu/drm/i915/i915_gem.c | 116
> ++--
> include/uapi/drm/i915_drm.h | 25
> 3 files changed, 164 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0d11cbe..f1aea86 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -498,6 +498,32 @@ static int i915_gem_object_info(struct seq_file *m,
> void* data)
> return 0;
> }
>
> +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_i915_gem_get_aperture arg;
> + int ret;
> +
> + ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
> + if (ret)
> + return ret;
> +
> + seq_printf(m, "Total size of the GTT: %llu bytes\n",
> +arg.aper_size);
> + seq_printf(m, "Available space in the GTT: %llu bytes\n",
> +arg.aper_available_size);
> + seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
> +arg.map_available_size);
> + seq_printf(m, "Single largest space in the mappable aperture: %llu
> bytes\n",
> +arg.map_largest_size);
> + seq_printf(m, "Available space for fences: %llu bytes\n",
> +arg.fence_available_size);
> + seq_printf(m, "Single largest fence available: %llu bytes\n",
> +arg.fence_largest_size);
> +
> + return 0;
> +}
> +
> static int i915_gem_gtt_info(struct seq_file *m, void *data)
> {
> struct drm_info_node *node = m->private;
> @@ -4398,6 +4424,7 @@ static int i915_debugfs_create(struct dentry *root,
> static const struct drm_info_list i915_debugfs_list[] = {
> {"i915_capabilities", i915_capabilities, 0},
> {"i915_gem_objects", i915_gem_object_info, 0},
> + {"i915_gem_aperture", i915_gem_aperture_info, 0},
> {"i915_gem_gtt", i915_gem_gtt_info, 0},
> {"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
> {"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 123ce34..0a074bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -31,6 +31,7 @@
> #include "i915_drv.h"
> #include "i915_trace.h"
> #include "intel_drv.h"
> +#include
> #include
> #include
> #include
> @@ -153,6 +154,55 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
> return 0;
> }
>
> +static inline bool
> +i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
> +{
> + return i915_gem_obj_bound_any(obj) && !obj->active;
> +}
> +
> +static int obj_rank_by_ggtt(void *priv,
> + struct list_head *A,
> + struct list_head *B)
> +{
> + struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
> + struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
> +
> + return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt
Re: [Intel-gfx] [PATCH 10/11] drm/i915: add irq_barrier operation for synchronising reads
On Wed, Jan 28, 2015 at 10:55:53AM +0100, Daniel Vetter wrote:
> On Mon, Jan 26, 2015 at 04:43:24AM -0800, Rodrigo Vivi wrote:
> > From: Dave Gordon
> >
> > On some generations of chips, it is necessary to read an MMIO register
> > before getting the sequence number from the status page in main memory,
> > in order to ensure coherency; and on all generations this should be
> > either helpful or harmless.
> >
> > In general, we want this operation to be the cheapest possible, since
> > we require only the side-effect of DMA completion and don't interpret
> > the result of the read, and don't require any coordination with other
> > threads, power domains, or anything else.
> >
> > However, finding a suitable register may be problematic; on GEN6 chips
> > the ACTHD register was used, but on VLV et al access to this register
> > requires FORCEWAKE and therefore many complications involving spinlocks
> > and polling.
> >
> > So this commit introduces this synchronising operation as a distinct
> > vfunc in the engine structure, so that it can be GEN- or chip-specific
> > if needed.
> >
> > And there are three implementations; a dummy one, for chips where no
> > synchronising read is needed, a gen6(+) version that issues a posting
> > read (to TAIL), and a VLV-specific one that issues a raw read instead,
> > avoiding touching FORCEWAKE and GTFIFO and other such complications.
> >
> > We then change gen6_ring_get_seqno() to use this new irq_barrier rather
> > than a POSTING_READ of ACTHD. Note that both older (pre-GEN6) and newer
> > (GEN8+) devices running in LRC mode do not currently include any posting
> > read in their own get_seqno() implementations, so this change only
> > makes a difference on VLV (and not CHV+).
> >
> > Signed-off-by: Dave Gordon
> > Tested-By: PRC QA PRTS (Patch Regression Test System Contact:
> > shuang...@intel.com)
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 37
> > +++--
> > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> > 2 files changed, 36 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 23020d6..97473ed 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1227,6 +1227,28 @@ pc_render_add_request(struct intel_engine_cs *ring)
> > return 0;
> > }
> >
> > +static void
> > +dummy_irq_barrier(struct intel_engine_cs *ring)
> > +{
> > +}
> > +
> > +static void
> > +gen6_irq_barrier(struct intel_engine_cs *ring)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(ring->dev);
> > + POSTING_READ(RING_TAIL(ring->mmio_base));
> > +}
> > +
> > +#define __raw_i915_read32(dev_priv__, reg__)
> > readl((dev_priv__)->regs + (reg__))
> > +#define RAW_POSTING_READ(reg__)
> > (void)__raw_i915_read32(dev_priv, reg__)
> > +
> > +static void
> > +vlv_irq_barrier(struct intel_engine_cs *ring)
> > +{
> > + struct drm_i915_private *dev_priv = to_i915(ring->dev);
> > + RAW_POSTING_READ(RING_TAIL(ring->mmio_base));
> > +}
> > +
> > static u32
> > gen6_ring_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency)
> > {
> > @@ -1234,8 +1256,7 @@ gen6_ring_get_seqno(struct intel_engine_cs *ring,
> > bool lazy_coherency)
> > * ivb (and maybe also on snb) by reading from a CS register (like
> > * ACTHD) before reading the status page. */
> > if (!lazy_coherency) {
> > - struct drm_i915_private *dev_priv = ring->dev->dev_private;
> > - POSTING_READ(RING_ACTHD(ring->mmio_base));
> > + ring->irq_barrier(ring);
> > }
>
> Imo just do a vlv_ring_get_seqno if this is a problem. Adding a vfunc with
> mostly empty or same implemenation to another very tiny vfunc isn't doing
> a whole lot of good to the codebase.
Or rather since there is just one place that cares about the
irq_barrier, just call it from that callsite and simplify get_seqno.
But the whole vlv is special argument still seems nebulous in the first
place.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support
On Wed, Jan 28, 2015 at 10:50:18AM +0100, Daniel Vetter wrote: > On Wed, Jan 28, 2015 at 09:23:46AM +, Chris Wilson wrote: > > On Wed, Jan 28, 2015 at 10:22:15AM +0100, Daniel Vetter wrote: > > > On Mon, Jan 26, 2015 at 09:08:03AM +, Chris Wilson wrote: > > > > On Mon, Jan 26, 2015 at 08:52:39AM +0100, Daniel Vetter wrote: > > > > > I think the problem will be platforms that want full explicit fence > > > > > (like > > > > > android) but allow delayed creation of the fence fd from a gl sync > > > > > object > > > > > (like the android egl extension allows). > > > > > > > > > > I'm not sure yet how to best expose that really since just creating a > > > > > fence from the implicit request attached to the batch might upset the > > > > > interface purists with the mix in implicit and explicit fencing ;-) > > > > > Hence > > > > > why I think for now we should just do the eager fd creation at execbuf > > > > > until ppl scream (well maybe not merge this patch until ppl scream > > > > > ...). > > > > > > > > Everything we do is buffer centric. Even in the future with random bits > > > > of memory, we will still use buffers behind the scenes. From an > > > > interface perspective, it is clearer to me if we say "give me a fence > > > > for > > > > this buffer". Exactly the same way as we say "is this buffer busy" or > > > > "wait on this buffer". The change is that we now hand back an fd to slot > > > > into an event loop. That, to me, is a much smaller evolutionary step of > > > > the existing API, and yet more versatile than just attaching one to the > > > > execbuf. > > > > > > The problem is that big parts of the world do not subscribe to that buffer > > > centric view of gfx. Imo since those parts will be the primary users of > > > this interface we should try to fit their ideas as much as feasible. Later > > > on (if we need it) we can add some glue to tie in the buffer-centric > > > implicit model with the explicit model. > > > > They won't be using execbuffer either. So what's your point? > > Android very much will user execbuffer. And even the in-flight buffered > svm stuff will keep on using execbuf (just without any relocs). So still buffer-centric and would benefit from the more general and more explict fencing rather than just execbuf. If you look at the throttling in mesa, you can already see a place that would rather create a fence on a buffer rather than its very approximate execbuf tracking. > And once we indeed can make the case (plus have the hw) for direct > userspace submission I think the kernel shouldn't be in the business of > creating fence objects: The ring will be fully under control of > userspace, and that's the only place we could insert a seqno into. So > again the same trust issues. No buffers, no requests, nothing for the kernel to do. No impact on designing an API that is useful today. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
On Wed, Jan 28, 2015 at 11:45:04AM +0200, Mika Kuoppala wrote:
> intel_uncore_early_sanitize() will reset the forcewake registers. When
> forcewake domains were introduced, the domain init was done after the
> sanitization of the forcewake registers. And as the resetting of
> registers use the domain accessors, we tried to reset the forcewake
> registers with unitialized forcewake domains and failed.
>
> Fix this by sanitizing after all the domains have been initialized.
> On ivb we need special care as there we need early forcewake access to
> determine the final configuration for the forcewake domain.
>
> This regression was introduced in
>
> commit 05a2fb157e44a53c79133805d30eaada43911941
> Author: Mika Kuoppala
> Date: Mon Jan 19 16:20:43 2015 +0200
>
> drm/i915: Consolidate forcewake code
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
> Reported-by: Olof Johansson
> Tested-by: Darren Hart
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index b3951f2..c438ca4 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private
> *dev_priv)
> static inline void
> fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
> {
> + WARN_ON(d->reg_set == 0);
> __raw_i915_write32(d->i915, d->reg_set, d->val_reset);
> }
>
> @@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum
> forcewake_domains fw_do
> struct intel_uncore_forcewake_domain *d;
> enum forcewake_domain_id id;
>
> + WARN_ON(dev_priv->uncore.fw_domains == 0);
> +
> for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
> fw_domain_reset(d);
>
> @@ -987,8 +990,7 @@ static void fw_domain_init(struct drm_i915_private
> *dev_priv,
> void intel_uncore_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - __intel_uncore_early_sanitize(dev, false);
> + bool sanitize_done = false;
I felt this looks quite clumsy. The only reason why you want to restrict
calling __intel_uncore_early_sanitize() is that it does ellc_size
detection and has a DRM_INFO right?
I think you want to pull that out of __intel_uncore_early_santize() into
intel_uncore_init() itself (or better, it's own
intel_uncore_ellc_detect()). ellc_size detection has nothing to do with
sanitizing register state.
Then it should be simple to enough to sanitize twice, once with a
comment in the code explaining how we verify that FORCEWAKE_MT is
enabled by a manual forcewaked read of ECOBUS.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Introduce intel_set_rps()
On Tue, Jan 27, 2015 at 04:36:16PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Replace the valleyview_set_rps() and gen6_set_rps() calls with
> intel_set_rps() which itself does the IS_VALLEYVIEW() check. The
> code becomes simpler since the callers don't have to do this check
> themselves.
>
> Most of the change was performe with the following semantic patch:
> @@
> expression E1, E2;
> @@
> (
> - valleyview_set_rps(E1, E2)
> + intel_set_rps(E1, E2)
> |
> - gen6_set_rps(E1, E2)
> + intel_set_rps(E1, E2)
> )
>
> @@
> expression E1, E2, E3;
> @@
> - if (IS_VALLEYVIEW(E1)) {
> - intel_set_rps(E2, E3);
> - } else {
> - intel_set_rps(E2, E3);
> - }
> + intel_set_rps(E2, E3);
>
> Adding intel_set_rps() and making valleyview_set_rps() and gen6_set_rps()
> static was done manually.
>
> Cc: Chris Wilson
> Suggested-by: Chris Wilson
> Signed-off-by: Ville Syrjälä
Hmm, I like half of them :|
The external callers from i915_debugfs.c, i915_irq.c are good. But
inside intel_pm.c, it is more mixed. gen6_rps_boost() clearly wants
intel_set_rps(), but from within the gen specific lowlevel functions, it
looks odder to callback into intel_set_rps.
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6ece663..2bad1e8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3750,7 +3750,7 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private
> *dev_priv, u8 val)
> /* gen6_set_rps is called to update the frequency request, but should also be
> * called when the range (min_delay and max_delay) is modified so that we can
> * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
> -void gen6_set_rps(struct drm_device *dev, u8 val)
> +static void gen6_set_rps(struct drm_device *dev, u8 val)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -3801,7 +3801,7 @@ static void vlv_set_rps_idle(struct drm_i915_private
> *dev_priv)
>
> /* CHV and latest VLV don't need to force the gfx clock */
> if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
> - valleyview_set_rps(dev_priv->dev,
> dev_priv->rps.min_freq_softlimit);
> + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> return;
> }
>
> @@ -3842,7 +3842,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
> if (IS_VALLEYVIEW(dev))
> vlv_set_rps_idle(dev_priv);
> else
> - gen6_set_rps(dev_priv->dev,
> dev_priv->rps.min_freq_softlimit);
> + intel_set_rps(dev_priv->dev,
> + dev_priv->rps.min_freq_softlimit);
> dev_priv->rps.last_adj = 0;
> }
> mutex_unlock(&dev_priv->rps.hw_lock);
These two are the most dubious for me.
> static void gen9_disable_rps(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4176,7 +4180,7 @@ static void gen8_enable_rps(struct drm_device *dev)
> /* 6: Ring frequency + overclocking (our driver does this later */
>
> dev_priv->rps.power = HIGH_POWER; /* force a reset */
> - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> }
> @@ -4270,7 +4274,7 @@ static void gen6_enable_rps(struct drm_device *dev)
> }
>
> dev_priv->rps.power = HIGH_POWER; /* force a reset */
> - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
>
> rc6vids = 0;
> ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> &rc6vids);
> @@ -4812,7 +4816,7 @@ static void cherryview_enable_rps(struct drm_device
> *dev)
>intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
>dev_priv->rps.efficient_freq);
>
> - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
> + intel_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
>
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> }
> @@ -4896,7 +4900,7 @@ static void valleyview_enable_rps(struct drm_device
> *dev)
>intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
>dev_priv->rps.efficient_freq);
>
> - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
> + intel_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
>
> intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> }
However, these should probably be converted to gen6_rps_idle() calls
instead (which requires setting dev_priv->rps.enable earlier) or movinng
the idle call out of the gen specific enable rps functions and into the
main intel_gen6_powersave_work().
-Chris
--
Chris Wilson, Intel Open Source Te
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Handle CHV in vlv_set_rps_idle()
On Tue, Jan 27, 2015 at 04:36:15PM +0200, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Move the CHV check into vlv_set_rps_idle() to simplify the caller a bit. > > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
On Wed, Jan 28, 2015 at 10:17:39AM +, Chris Wilson wrote:
> On Wed, Jan 28, 2015 at 11:45:04AM +0200, Mika Kuoppala wrote:
> > intel_uncore_early_sanitize() will reset the forcewake registers. When
> > forcewake domains were introduced, the domain init was done after the
> > sanitization of the forcewake registers. And as the resetting of
> > registers use the domain accessors, we tried to reset the forcewake
> > registers with unitialized forcewake domains and failed.
> >
> > Fix this by sanitizing after all the domains have been initialized.
> > On ivb we need special care as there we need early forcewake access to
> > determine the final configuration for the forcewake domain.
> >
> > This regression was introduced in
> >
> > commit 05a2fb157e44a53c79133805d30eaada43911941
> > Author: Mika Kuoppala
> > Date: Mon Jan 19 16:20:43 2015 +0200
> >
> > drm/i915: Consolidate forcewake code
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
> > Reported-by: Olof Johansson
> > Tested-by: Darren Hart
> > Signed-off-by: Mika Kuoppala
> > ---
> > drivers/gpu/drm/i915/intel_uncore.c | 11 +--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> > b/drivers/gpu/drm/i915/intel_uncore.c
> > index b3951f2..c438ca4 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private
> > *dev_priv)
> > static inline void
> > fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
> > {
> > + WARN_ON(d->reg_set == 0);
> > __raw_i915_write32(d->i915, d->reg_set, d->val_reset);
> > }
> >
> > @@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv,
> > enum forcewake_domains fw_do
> > struct intel_uncore_forcewake_domain *d;
> > enum forcewake_domain_id id;
> >
> > + WARN_ON(dev_priv->uncore.fw_domains == 0);
> > +
> > for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
> > fw_domain_reset(d);
> >
> > @@ -987,8 +990,7 @@ static void fw_domain_init(struct drm_i915_private
> > *dev_priv,
> > void intel_uncore_init(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> > - __intel_uncore_early_sanitize(dev, false);
> > + bool sanitize_done = false;
>
> I felt this looks quite clumsy. The only reason why you want to restrict
> calling __intel_uncore_early_sanitize() is that it does ellc_size
> detection and has a DRM_INFO right?
>
> I think you want to pull that out of __intel_uncore_early_santize() into
> intel_uncore_init() itself (or better, it's own
> intel_uncore_ellc_detect()). ellc_size detection has nothing to do with
> sanitizing register state.
>
> Then it should be simple to enough to sanitize twice, once with a
> comment in the code explaining how we verify that FORCEWAKE_MT is
> enabled by a manual forcewaked read of ECOBUS.
Also, why are we not calling fw_domain_reset() from fw_domain_init()?
That would be enough to avoid the early santize required for ivb, right?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
Chris Wilson writes:
> On Wed, Jan 28, 2015 at 10:17:39AM +, Chris Wilson wrote:
>> On Wed, Jan 28, 2015 at 11:45:04AM +0200, Mika Kuoppala wrote:
>> > intel_uncore_early_sanitize() will reset the forcewake registers. When
>> > forcewake domains were introduced, the domain init was done after the
>> > sanitization of the forcewake registers. And as the resetting of
>> > registers use the domain accessors, we tried to reset the forcewake
>> > registers with unitialized forcewake domains and failed.
>> >
>> > Fix this by sanitizing after all the domains have been initialized.
>> > On ivb we need special care as there we need early forcewake access to
>> > determine the final configuration for the forcewake domain.
>> >
>> > This regression was introduced in
>> >
>> > commit 05a2fb157e44a53c79133805d30eaada43911941
>> > Author: Mika Kuoppala
>> > Date: Mon Jan 19 16:20:43 2015 +0200
>> >
>> > drm/i915: Consolidate forcewake code
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
>> > Reported-by: Olof Johansson
>> > Tested-by: Darren Hart
>> > Signed-off-by: Mika Kuoppala
>> > ---
>> > drivers/gpu/drm/i915/intel_uncore.c | 11 +--
>> > 1 file changed, 9 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> > b/drivers/gpu/drm/i915/intel_uncore.c
>> > index b3951f2..c438ca4 100644
>> > --- a/drivers/gpu/drm/i915/intel_uncore.c
>> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> > @@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private
>> > *dev_priv)
>> > static inline void
>> > fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
>> > {
>> > + WARN_ON(d->reg_set == 0);
>> >__raw_i915_write32(d->i915, d->reg_set, d->val_reset);
>> > }
>> >
>> > @@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv,
>> > enum forcewake_domains fw_do
>> >struct intel_uncore_forcewake_domain *d;
>> >enum forcewake_domain_id id;
>> >
>> > + WARN_ON(dev_priv->uncore.fw_domains == 0);
>> > +
>> >for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
>> >fw_domain_reset(d);
>> >
>> > @@ -987,8 +990,7 @@ static void fw_domain_init(struct drm_i915_private
>> > *dev_priv,
>> > void intel_uncore_init(struct drm_device *dev)
>> > {
>> >struct drm_i915_private *dev_priv = dev->dev_private;
>> > -
>> > - __intel_uncore_early_sanitize(dev, false);
>> > + bool sanitize_done = false;
>>
>> I felt this looks quite clumsy. The only reason why you want to restrict
>> calling __intel_uncore_early_sanitize() is that it does ellc_size
>> detection and has a DRM_INFO right?
>>
>> I think you want to pull that out of __intel_uncore_early_santize() into
>> intel_uncore_init() itself (or better, it's own
>> intel_uncore_ellc_detect()). ellc_size detection has nothing to do with
>> sanitizing register state.
>>
>> Then it should be simple to enough to sanitize twice, once with a
>> comment in the code explaining how we verify that FORCEWAKE_MT is
>> enabled by a manual forcewaked read of ECOBUS.
>
> Also, why are we not calling fw_domain_reset() from fw_domain_init()?
> That would be enough to avoid the early santize required for ivb,
> right?
Agreed here. That was my plan originally, doing the sanitize inside
in domain inits. But I wanted to fix this particular item by trying to
be as close as possible to the previous init/forcewake ordering on all gens.
Reasoning is that I would like to see this stabilize a short while
before introducing further changes. I burned my fingers already touching
these, so they need to heal :)
Ok if this is for future work?
-Mika
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] dma-buf: cleanup dma_buf_export() to make it easily extensible
On 28/01/15 06:00, Sumit Semwal wrote:
> At present, dma_buf_export() takes a series of parameters, which
> makes it difficult to add any new parameters for exporters, if required.
>
> Make it simpler by moving all these parameters into a struct, and pass
> the struct * as parameter to dma_buf_export().
>
> While at it, unite dma_buf_export_named() with dma_buf_export(), and
> change all callers accordingly.
>
> Signed-off-by: Sumit Semwal
> ---
> v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default
>
> drivers/dma-buf/dma-buf.c | 47
> +-
> drivers/gpu/drm/armada/armada_gem.c| 10 --
> drivers/gpu/drm/drm_prime.c| 12 ---
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++--
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 -
> drivers/gpu/drm/tegra/gem.c| 10 --
> drivers/gpu/drm/ttm/ttm_object.c | 9 +++--
> drivers/gpu/drm/udl/udl_dmabuf.c | 9 -
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 -
> drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 -
> drivers/media/v4l2-core/videobuf2-vmalloc.c| 8 -
> drivers/staging/android/ion/ion.c | 9 +++--
> include/linux/dma-buf.h| 35 +++
> 14 files changed, 143 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 5be225c2ba98..6d3df3dd9310 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
> }
>
> /**
> - * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
> + * dma_buf_export - Creates a new dma_buf, and associates an anon file
> * with this buffer, so it can be exported.
> * Also connect the allocator specific data and ops to the buffer.
> * Additionally, provide a name string for exporter; useful in debugging.
> @@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
> * @exp_name:[in]name of the exporting module - useful for
> debugging.
> * @resv:[in]reservation-object, NULL to allocate default one.
> *
> + * All the above info comes from struct dma_buf_export_info.
> + *
> * Returns, on success, a newly created dma_buf object, which wraps the
> * supplied private data and operations for dma_buf_ops. On either missing
> * ops, or error in allocating struct dma_buf, will return negative error.
> *
> */
> -struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops
> *ops,
> - size_t size, int flags, const char *exp_name,
> - struct reservation_object *resv)
> +struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
> {
> struct dma_buf *dmabuf;
> struct file *file;
> size_t alloc_size = sizeof(struct dma_buf);
> - if (!resv)
> + if (!exp_info->resv)
> alloc_size += sizeof(struct reservation_object);
> else
> /* prevent &dma_buf[1] == dma_buf->resv */
> alloc_size += 1;
>
> - if (WARN_ON(!priv || !ops
> - || !ops->map_dma_buf
> - || !ops->unmap_dma_buf
> - || !ops->release
> - || !ops->kmap_atomic
> - || !ops->kmap
> - || !ops->mmap)) {
> + if (WARN_ON(!exp_info->priv
> + || !exp_info->ops
> + || !exp_info->ops->map_dma_buf
> + || !exp_info->ops->unmap_dma_buf
> + || !exp_info->ops->release
> + || !exp_info->ops->kmap_atomic
> + || !exp_info->ops->kmap
> + || !exp_info->ops->mmap)) {
> return ERR_PTR(-EINVAL);
> }
>
> @@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const
> struct dma_buf_ops *ops,
> if (dmabuf == NULL)
> return ERR_PTR(-ENOMEM);
>
> - dmabuf->priv = priv;
> - dmabuf->ops = ops;
> - dmabuf->size = size;
> - dmabuf->exp_name = exp_name;
> + dmabuf->priv = exp_info->priv;
> + dmabuf->ops = exp_info->ops;
> + dmabuf->size = exp_info->size;
> + dmabuf->exp_name = exp_info->exp_name;
> init_waitqueue_head(&dmabuf->poll);
> dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = &dmabuf->poll;
> dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
>
> - if (!resv) {
> - resv = (struct reservation_object *)&dmabuf[1];
> - reservation_object_init(resv);
> + if (!exp_info->resv) {
> + exp_info->resv = (struct reservation_object *)&dmabuf[1];
> + reservation_object_init(e
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
> On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
> > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
> > this WARN at boot (and pretty frequently afterwards):
> >
> > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377
> > gen6_set_rps+0x371/0x3c0()
> > WARN_ON(val > dev_priv->rps.max_freq_softlimit)
>
> [snip]
>
> > I'm not at all familiar with this hardware, but I took a quick look into
> > what changed with this commit for my laptop. Before the commit,
> > rps.min_freq_softlimit is 4 (from rps.min_freq) and
> > rps.max_freq_softlimit is 22.
> >
> > After the commit, rps.min_freq_softlimit is set to the
> > rps.efficient_freq value read from pcode, which is 34 on my laptop.
> > So later when gen6_set_rps() is called with rps.min_freq_softlimit that
> > warning is hit.
> >
> > Any thoughts? It certainly seems fishy that this commit causes
> > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
>
> Very fishy indeed. Moral of this story, never trust hw.
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3e630feb18e4..bbedd2901c54 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct
> drm_device *dev)
> &ddcc_status);
> if (0 == ret)
> dev_priv->rps.efficient_freq =
> - (ddcc_status >> 8) & 0xff;
> + clamp_t(u8,
> + (ddcc_status >> 8) & 0xff,
> + dev_priv->rps.min_freq,
> + dev_priv->rps.max_freq);
Maybe better to fall back to rp1_freq if this is bogus?
> }
>
> /* Preserve min/max settings in case of re-init */
>
> But really it is probably just best to disable the query for hsw:
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3e630feb18e4..01bd508e81f6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct drm_device
> *dev)
> dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
>
> dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> + if (IS_BROADWELL(dev)) {
> ret = sandybridge_pcode_read(dev_priv,
> HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> &ddcc_status);
>
> Paranoia says we do both.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
On Wed, Jan 28, 2015 at 12:59:52PM +0200, Mika Kuoppala wrote: > Chris Wilson writes: > > Also, why are we not calling fw_domain_reset() from fw_domain_init()? > > That would be enough to avoid the early santize required for ivb, > > right? > > Agreed here. That was my plan originally, doing the sanitize inside > in domain inits. But I wanted to fix this particular item by trying to > be as close as possible to the previous init/forcewake ordering on all gens. > > Reasoning is that I would like to see this stabilize a short while > before introducing further changes. I burned my fingers already touching > these, so they need to heal :) > > Ok if this is for future work? The bug is in -next, so take advantage of the guinea pigs... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Be consistent on printing seqnos
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5645 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 353/353 352/353 ILK 353/353 353/353 SNB -1 400/422 399/422 IVB +2-2 485/487 485/487 BYT 296/296 296/296 HSW +1-1 507/508 507/508 BDW 401/402 401/402 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gen3_render_linear_blits PASS(3, M25M23) CRASH(1, M23) *SNB igt_gem_concurrent_blit_gpu-bcs-gpu-read-after-write PASS(2, M35) NO_RESULT(1, M35) *IVB igt_gem_persistent_relocs_forked-thrashing PASS(2, M34M4) NO_RESULT(1, M4) IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(2, M34)PASS(3, M4) PASS(1, M4) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(2, M34M4)PASS(5, M34M4M21) PASS(1, M4) *IVB igt_gem_storedw_loop_blt PASS(2, M34M4) DMESG_WARN(1, M4) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(1, M40)PASS(7, M40M20) PASS(1, M40) *HSW igt_pm_rpm_debugfs-read PASS(2, M40) DMESG_WARN(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] dma-buf: cleanup dma_buf_export() to make it easily extensible
On 28 January 2015 at 16:50, Daniel Thompson wrote:
> On 28/01/15 06:00, Sumit Semwal wrote:
>> +/**
>> + * helper macro for exporters; zeros and fills in most common values
>> + */
>> +#define DEFINE_DMA_BUF_EXPORT_INFO(a)\
>> + struct dma_buf_export_info a = {0}; \
>> + exp_info.exp_name = KBUILD_MODNAME
>> +
>
> This risks generating C99 warnings unless used with care (and only once
> per function). Shouldn't this be more like:
>
> #define DEFINE_DMA_BUF_EXPORT_INFO(a) \
> struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }
>
Ah! My bad; thanks for catching this, Daniel; I'll send out the
updated patch in a minute!
> Daniel.
>
--
Thanks and regards,
Sumit Semwal
Kernel Team Lead - Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Do only one posting read on forcewake get sequence
Follow the same semantics as in put side where we go through
all domains before doing posting read.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index ebd9068..d2423e6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -144,9 +144,10 @@ fw_domains_get(struct drm_i915_private *dev_priv, enum
forcewake_domains fw_doma
for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
fw_domain_wait_ack_clear(d);
fw_domain_get(d);
- fw_domain_posting_read(d);
fw_domain_wait_ack(d);
}
+
+ fw_domains_posting_read(dev_priv);
}
static void
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
commit 05a2fb157e44a53c79133805d30eaada43911941
Author: Mika Kuoppala
Date: Mon Jan 19 16:20:43 2015 +0200
drm/i915: Consolidate forcewake code
introduced domain handling where each domain has it's own posting
read registers. This changed the forcewake sequence on 'put' side when
there is multiple domains as there would be extra read between the domain
puts. Any posting read should be enough to flush all the changes.
Do a posting read only once, at the end of the sequence and for
the first domain. Like it was before.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index be2c7fc..ebd9068 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -123,42 +123,42 @@ fw_domain_posting_read(const struct
intel_uncore_forcewake_domain *d)
}
static void
-fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains
fw_domains)
+fw_domains_posting_read(struct drm_i915_private *dev_priv)
{
struct intel_uncore_forcewake_domain *d;
enum forcewake_domain_id id;
- for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
- fw_domain_wait_ack_clear(d);
- fw_domain_get(d);
+ /* No need to do for all, just do for first found */
+ for_each_fw_domain(d, dev_priv, id) {
fw_domain_posting_read(d);
- fw_domain_wait_ack(d);
+ break;
}
}
static void
-fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains
fw_domains)
+fw_domains_get(struct drm_i915_private *dev_priv, enum forcewake_domains
fw_domains)
{
struct intel_uncore_forcewake_domain *d;
enum forcewake_domain_id id;
for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
- fw_domain_put(d);
+ fw_domain_wait_ack_clear(d);
+ fw_domain_get(d);
fw_domain_posting_read(d);
+ fw_domain_wait_ack(d);
}
}
static void
-fw_domains_posting_read(struct drm_i915_private *dev_priv)
+fw_domains_put(struct drm_i915_private *dev_priv, enum forcewake_domains
fw_domains)
{
struct intel_uncore_forcewake_domain *d;
enum forcewake_domain_id id;
- /* No need to do for all, just do for first found */
- for_each_fw_domain(d, dev_priv, id) {
- fw_domain_posting_read(d);
- break;
- }
+ for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
+ fw_domain_put(d);
+
+ fw_domains_posting_read(dev_priv);
}
static void
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: Do uncore early sanitize after domain init
intel_uncore_early_sanitize() will reset the forcewake registers. When
forcewake domains were introduced, the domain init was done after the
sanitization of the forcewake registers. And as the resetting of
registers use the domain accessors, we tried to reset the forcewake
registers with unitialized forcewake domains and failed.
Fix this by sanitizing after all the domains have been initialized. Do
per domain clearing of forcewake register on domain init so that
IVB can do early access to ECOBUS do determine the final configuration.
This regression was introduced in
commit 05a2fb157e44a53c79133805d30eaada43911941
Author: Mika Kuoppala
Date: Mon Jan 19 16:20:43 2015 +0200
drm/i915: Consolidate forcewake code
v2: Carve out ellc detect, fw_domain_reset for ivb/ecobus (Chris)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805
Cc: Chris Wilson
Reported-by: Olof Johansson
Tested-by: Darren Hart (v1)
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 38 +
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index b3951f2..be2c7fc 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -72,6 +72,7 @@ assert_device_not_suspended(struct drm_i915_private *dev_priv)
static inline void
fw_domain_reset(const struct intel_uncore_forcewake_domain *d)
{
+ WARN_ON(d->reg_set == 0);
__raw_i915_write32(d->i915, d->reg_set, d->val_reset);
}
@@ -166,6 +167,8 @@ fw_domains_reset(struct drm_i915_private *dev_priv, enum
forcewake_domains fw_do
struct intel_uncore_forcewake_domain *d;
enum forcewake_domain_id id;
+ WARN_ON(dev_priv->uncore.fw_domains == 0);
+
for_each_fw_domain_mask(d, fw_domains, dev_priv, id)
fw_domain_reset(d);
@@ -321,14 +324,10 @@ void intel_uncore_forcewake_reset(struct drm_device *dev,
bool restore)
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
}
-static void __intel_uncore_early_sanitize(struct drm_device *dev,
- bool restore_forcewake)
+static void intel_uncore_ellc_detect(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- if (HAS_FPGA_DBG_UNCLAIMED(dev))
- __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
-
if ((IS_HASWELL(dev) || IS_BROADWELL(dev)) &&
(__raw_i915_read32(dev_priv, HSW_EDRAM_PRESENT) == 1)) {
/* The docs do not explain exactly how the calculation can be
@@ -339,6 +338,15 @@ static void __intel_uncore_early_sanitize(struct
drm_device *dev,
dev_priv->ellc_size = 128;
DRM_INFO("Found %zuMB of eLLC\n", dev_priv->ellc_size);
}
+}
+
+static void __intel_uncore_early_sanitize(struct drm_device *dev,
+ bool restore_forcewake)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (HAS_FPGA_DBG_UNCLAIMED(dev))
+ __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
/* clear out old GT FIFO errors */
if (IS_GEN6(dev) || IS_GEN7(dev))
@@ -982,14 +990,14 @@ static void fw_domain_init(struct drm_i915_private
*dev_priv,
setup_timer(&d->timer, intel_uncore_fw_release_timer, (unsigned long)d);
dev_priv->uncore.fw_domains |= (1 << domain_id);
+
+ fw_domain_reset(d);
}
-void intel_uncore_init(struct drm_device *dev)
+static void intel_uncore_fw_domains_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- __intel_uncore_early_sanitize(dev, false);
-
if (IS_GEN9(dev)) {
dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
@@ -1035,8 +1043,13 @@ void intel_uncore_init(struct drm_device *dev)
dev_priv->uncore.funcs.force_wake_put =
fw_domains_put_with_fifo;
+ /* We need to init first for ECOBUS access and then
+* determine later if we want to reinit, in case of MT access is
+* not working
+*/
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE_MT, FORCEWAKE_MT_ACK);
+
mutex_lock(&dev->struct_mutex);
fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
ecobus = __raw_i915_read32(dev_priv, ECOBUS);
@@ -1057,6 +1070,15 @@ void intel_uncore_init(struct drm_device *dev)
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
FORCEWAKE, FORCEWAKE_ACK);
}
+}
+
+void intel_uncore_init(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ intel_uncore_ellc_detect(dev)
Re: [Intel-gfx] [PATCH] drm/i915: Do uncore early sanitize after domain init
Chris Wilson writes: > On Wed, Jan 28, 2015 at 12:59:52PM +0200, Mika Kuoppala wrote: >> Chris Wilson writes: >> > Also, why are we not calling fw_domain_reset() from fw_domain_init()? >> > That would be enough to avoid the early santize required for ivb, >> > right? >> >> Agreed here. That was my plan originally, doing the sanitize inside >> in domain inits. But I wanted to fix this particular item by trying to >> be as close as possible to the previous init/forcewake ordering on all gens. >> >> Reasoning is that I would like to see this stabilize a short while >> before introducing further changes. I burned my fingers already touching >> these, so they need to heal :) >> >> Ok if this is for future work? > > The bug is in -next, so take advantage of the guinea pigs... v2 in: 1422449006-4028-1-git-send-email-mika.kuopp...@intel.com -Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible
At present, dma_buf_export() takes a series of parameters, which
makes it difficult to add any new parameters for exporters, if required.
Make it simpler by moving all these parameters into a struct, and pass
the struct * as parameter to dma_buf_export().
While at it, unite dma_buf_export_named() with dma_buf_export(), and
change all callers accordingly.
Signed-off-by: Sumit Semwal
---
v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using
{.exp_name = xxx} instead.
v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default
drivers/dma-buf/dma-buf.c | 47 +-
drivers/gpu/drm/armada/armada_gem.c| 10 --
drivers/gpu/drm/drm_prime.c| 12 ---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++--
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 -
drivers/gpu/drm/tegra/gem.c| 10 --
drivers/gpu/drm/ttm/ttm_object.c | 9 +++--
drivers/gpu/drm/udl/udl_dmabuf.c | 9 -
drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 -
drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 -
drivers/media/v4l2-core/videobuf2-vmalloc.c| 8 -
drivers/staging/android/ion/ion.c | 9 +++--
include/linux/dma-buf.h| 34 +++
14 files changed, 142 insertions(+), 50 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 5be225c2ba98..6d3df3dd9310 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
}
/**
- * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
+ * dma_buf_export - Creates a new dma_buf, and associates an anon file
* with this buffer, so it can be exported.
* Also connect the allocator specific data and ops to the buffer.
* Additionally, provide a name string for exporter; useful in debugging.
@@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
* @exp_name: [in]name of the exporting module - useful for debugging.
* @resv: [in]reservation-object, NULL to allocate default one.
*
+ * All the above info comes from struct dma_buf_export_info.
+ *
* Returns, on success, a newly created dma_buf object, which wraps the
* supplied private data and operations for dma_buf_ops. On either missing
* ops, or error in allocating struct dma_buf, will return negative error.
*
*/
-struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
- size_t size, int flags, const char *exp_name,
- struct reservation_object *resv)
+struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
{
struct dma_buf *dmabuf;
struct file *file;
size_t alloc_size = sizeof(struct dma_buf);
- if (!resv)
+ if (!exp_info->resv)
alloc_size += sizeof(struct reservation_object);
else
/* prevent &dma_buf[1] == dma_buf->resv */
alloc_size += 1;
- if (WARN_ON(!priv || !ops
- || !ops->map_dma_buf
- || !ops->unmap_dma_buf
- || !ops->release
- || !ops->kmap_atomic
- || !ops->kmap
- || !ops->mmap)) {
+ if (WARN_ON(!exp_info->priv
+ || !exp_info->ops
+ || !exp_info->ops->map_dma_buf
+ || !exp_info->ops->unmap_dma_buf
+ || !exp_info->ops->release
+ || !exp_info->ops->kmap_atomic
+ || !exp_info->ops->kmap
+ || !exp_info->ops->mmap)) {
return ERR_PTR(-EINVAL);
}
@@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const
struct dma_buf_ops *ops,
if (dmabuf == NULL)
return ERR_PTR(-ENOMEM);
- dmabuf->priv = priv;
- dmabuf->ops = ops;
- dmabuf->size = size;
- dmabuf->exp_name = exp_name;
+ dmabuf->priv = exp_info->priv;
+ dmabuf->ops = exp_info->ops;
+ dmabuf->size = exp_info->size;
+ dmabuf->exp_name = exp_info->exp_name;
init_waitqueue_head(&dmabuf->poll);
dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = &dmabuf->poll;
dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
- if (!resv) {
- resv = (struct reservation_object *)&dmabuf[1];
- reservation_object_init(resv);
+ if (!exp_info->resv) {
+ exp_info->resv = (struct reservation_object *)&dmabuf[1];
+ reservation_object_init(exp_info->resv);
}
- dmabuf->resv = resv;
+ dmabuf->resv
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: > commit 05a2fb157e44a53c79133805d30eaada43911941 > Author: Mika Kuoppala > Date: Mon Jan 19 16:20:43 2015 +0200 > > drm/i915: Consolidate forcewake code > > introduced domain handling where each domain has it's own posting > read registers. This changed the forcewake sequence on 'put' side when > there is multiple domains as there would be extra read between the domain > puts. Any posting read should be enough to flush all the changes. > > Do a posting read only once, at the end of the sequence and for > the first domain. Like it was before. > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala fwiw, I would argue that the posting read in _get() is superfluous as we will serialise the fw with not only the ack, but any subsequent mmio. On the _put() side we do want to flush the write so that the hw can power down as early as possible. So just kill the posting read from _get and otherwise drop the patch. :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Do only one posting read on forcewake get sequence
On Wed, Jan 28, 2015 at 02:43:26PM +0200, Mika Kuoppala wrote: > Follow the same semantics as in put side where we go through > all domains before doing posting read. > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala Having just argued that I don't want a posting read here at all (and that the existing one is just overly paranoid even by our standards), we can drop this patch as well :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Do uncore early sanitize after domain init
On Wed, Jan 28, 2015 at 02:43:24PM +0200, Mika Kuoppala wrote: > intel_uncore_early_sanitize() will reset the forcewake registers. When > forcewake domains were introduced, the domain init was done after the > sanitization of the forcewake registers. And as the resetting of > registers use the domain accessors, we tried to reset the forcewake > registers with unitialized forcewake domains and failed. > > Fix this by sanitizing after all the domains have been initialized. Do > per domain clearing of forcewake register on domain init so that > IVB can do early access to ECOBUS do determine the final configuration. > > This regression was introduced in > > commit 05a2fb157e44a53c79133805d30eaada43911941 > Author: Mika Kuoppala > Date: Mon Jan 19 16:20:43 2015 +0200 > > drm/i915: Consolidate forcewake code > > v2: Carve out ellc detect, fw_domain_reset for ivb/ecobus (Chris) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88805 > Cc: Chris Wilson > Reported-by: Olof Johansson > Tested-by: Darren Hart (v1) > Signed-off-by: Mika Kuoppala Looks good to me, I hope it survives booting... Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Don't do posting reads on getting forcewake
The checking for ack and also any subsequent mmio access
will serialize with setting the forcewake bit. Drop the
posting read as superfluous.
Note that in the put side we still want to keep the posting read
as it will ensure that the hw sees our forcewake release in a
timely manner and doesn't keep the hw powered up.
Suggested-by: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index be2c7fc..76b60a3 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -131,7 +131,6 @@ fw_domains_get(struct drm_i915_private *dev_priv, enum
forcewake_domains fw_doma
for_each_fw_domain_mask(d, fw_domains, dev_priv, id) {
fw_domain_wait_ack_clear(d);
fw_domain_get(d);
- fw_domain_posting_read(d);
fw_domain_wait_ack(d);
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible
Hi Mauro,
On 28 January 2015 at 18:53, Mauro Carvalho Chehab
wrote:
> Em Wed, 28 Jan 2015 18:24:03 +0530
> Sumit Semwal escreveu:
>
>> +/**
>> + * helper macro for exporters; zeros and fills in most common values
>> + */
>> +#define DEFINE_DMA_BUF_EXPORT_INFO(a)\
>> + struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }
>> +
>
> I suspect that this will let the other fields not initialized.
>
> You likely need to do:
>
> #define DEFINE_DMA_BUF_EXPORT_INFO(a) \
> struct dma_buf_export_info a = {\
> .exp_name = KBUILD_MODNAME; \
> .fields = 0;\
> ...
> }
I suspected the same, but Daniel kindly referred to the C99 standard,
which states:
" If there are fewer initializers in a brace-enclosed list than there
are elements or members
of an aggregate, or fewer characters in a string literal used to
initialize an array of known
size than there are elements in the array, the remainder of the
aggregate shall be
initialized implicitly the same as objects that have static storage duration."
So I think we're well covered there?
>
> Regards,
> Mauro
--
Thanks and regards,
Sumit Semwal
Kernel Team Lead - Linaro Mobile Group
Linaro.org │ Open source software for ARM SoCs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
Chris Wilson writes: > On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: >> commit 05a2fb157e44a53c79133805d30eaada43911941 >> Author: Mika Kuoppala >> Date: Mon Jan 19 16:20:43 2015 +0200 >> >> drm/i915: Consolidate forcewake code >> >> introduced domain handling where each domain has it's own posting >> read registers. This changed the forcewake sequence on 'put' side when >> there is multiple domains as there would be extra read between the domain >> puts. Any posting read should be enough to flush all the changes. >> >> Do a posting read only once, at the end of the sequence and for >> the first domain. Like it was before. >> >> Cc: Chris Wilson >> Signed-off-by: Mika Kuoppala > > fwiw, I would argue that the posting read in _get() is superfluous as we > will serialise the fw with not only the ack, but any subsequent mmio. > > On the _put() side we do want to flush the write so that the hw can > power down as early as possible. So just kill the posting read from _get > and otherwise drop the patch. :) Yes, both put/get patches should be dropped. I posted a patch removing the posting read on get side and with your explanations in commit message. This all starts to make so much sense that some gen is bound to break ;) -Mika > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
On Wed, Jan 28, 2015 at 03:28:56PM +0200, Mika Kuoppala wrote: > Chris Wilson writes: > > > On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: > >> commit 05a2fb157e44a53c79133805d30eaada43911941 > >> Author: Mika Kuoppala > >> Date: Mon Jan 19 16:20:43 2015 +0200 > >> > >> drm/i915: Consolidate forcewake code > >> > >> introduced domain handling where each domain has it's own posting > >> read registers. This changed the forcewake sequence on 'put' side when > >> there is multiple domains as there would be extra read between the domain > >> puts. Any posting read should be enough to flush all the changes. > >> > >> Do a posting read only once, at the end of the sequence and for > >> the first domain. Like it was before. > >> > >> Cc: Chris Wilson > >> Signed-off-by: Mika Kuoppala > > > > fwiw, I would argue that the posting read in _get() is superfluous as we > > will serialise the fw with not only the ack, but any subsequent mmio. > > > > On the _put() side we do want to flush the write so that the hw can > > power down as early as possible. So just kill the posting read from _get > > and otherwise drop the patch. :) > > Yes, both put/get patches should be dropped. I posted a patch removing > the posting read on get side and with your explanations in commit message. > > This all starts to make so much sense that some gen is bound to break ;) IIRC the posting read from same cache line actually fixed real bugs. So I'm a bit worried about dropping them. But I suppose it's possible only the _put side was important for those bugs. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't do posting reads on getting forcewake
On Wed, Jan 28, 2015 at 03:25:05PM +0200, Mika Kuoppala wrote: > The checking for ack and also any subsequent mmio access > will serialize with setting the forcewake bit. Drop the > posting read as superfluous. > > Note that in the put side we still want to keep the posting read > as it will ensure that the hw sees our forcewake release in a > timely manner and doesn't keep the hw powered up. > > Suggested-by: Chris Wilson > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] memcontrol.c BUG
On Wed 28-01-15 08:48:52, Chris Wilson wrote:
> On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1165369
> >
> > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2
> > mapcount:0 mapping: (null) index:0x0
> > Nov 18 09:23:22 elissa.gathman.org kernel: page flags:
> > 0x80090029(locked|uptodate|lru|swapcache|swapbacked)
> > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because:
> > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage))
> > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here
> > ]
> > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at
> > mm/memcontrol.c:6733!
I guess this matches the following bugon in your kernel:
VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage);
so the oldpage is on the LRU list already. I am completely unfamiliar
with 965GM but is the page perhaps shared with somebody with a different
gfp mask requirement (e.g. userspace accessing the memory via mmap)? So
the other (racing) caller didn't need to move the page and put it on
LRU.
If yes we need to tell shmem_replace_page to do the lrucare handling.
diff --git a/mm/shmem.c b/mm/shmem.c
index 339e06639956..e3cdc1a16c0f 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1013,7 +1013,7 @@ static int shmem_replace_page(struct page **pagep, gfp_t
gfp,
*/
oldpage = newpage;
} else {
- mem_cgroup_migrate(oldpage, newpage, false);
+ mem_cgroup_migrate(oldpage, newpage, true);
lru_cache_add_anon(newpage);
*pagep = newpage;
}
[...]
> 965GM and that it uniquely uses
>
> mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> /* 965gm cannot relocate objects above 4GiB. */
> mask &= ~__GFP_HIGHMEM;
> mask |= __GFP_DMA32;
> }
--
Michal Hocko
SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible
Op 28-01-15 om 13:54 schreef Sumit Semwal:
> At present, dma_buf_export() takes a series of parameters, which
> makes it difficult to add any new parameters for exporters, if required.
>
> Make it simpler by moving all these parameters into a struct, and pass
> the struct * as parameter to dma_buf_export().
>
> While at it, unite dma_buf_export_named() with dma_buf_export(), and
> change all callers accordingly.
>
> Signed-off-by: Sumit Semwal
> ---
> v3: Daniel Thompson caught the C99 warning issue w/ using {0}; using
> {.exp_name = xxx} instead.
>
> v2: add macro to zero out local struct, and fill KBUILD_MODNAME by default
>
> drivers/dma-buf/dma-buf.c | 47
> +-
> drivers/gpu/drm/armada/armada_gem.c| 10 --
> drivers/gpu/drm/drm_prime.c| 12 ---
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 9 +++--
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 --
> drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c | 9 -
> drivers/gpu/drm/tegra/gem.c| 10 --
> drivers/gpu/drm/ttm/ttm_object.c | 9 +++--
> drivers/gpu/drm/udl/udl_dmabuf.c | 9 -
> drivers/media/v4l2-core/videobuf2-dma-contig.c | 8 -
> drivers/media/v4l2-core/videobuf2-dma-sg.c | 8 -
> drivers/media/v4l2-core/videobuf2-vmalloc.c| 8 -
> drivers/staging/android/ion/ion.c | 9 +++--
> include/linux/dma-buf.h| 34 +++
> 14 files changed, 142 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 5be225c2ba98..6d3df3dd9310 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -265,7 +265,7 @@ static inline int is_dma_buf_file(struct file *file)
> }
>
> /**
> - * dma_buf_export_named - Creates a new dma_buf, and associates an anon file
> + * dma_buf_export - Creates a new dma_buf, and associates an anon file
> * with this buffer, so it can be exported.
> * Also connect the allocator specific data and ops to the buffer.
> * Additionally, provide a name string for exporter; useful in debugging.
> @@ -277,31 +277,32 @@ static inline int is_dma_buf_file(struct file *file)
> * @exp_name:[in]name of the exporting module - useful for
> debugging.
> * @resv:[in]reservation-object, NULL to allocate default one.
> *
> + * All the above info comes from struct dma_buf_export_info.
> + *
> * Returns, on success, a newly created dma_buf object, which wraps the
> * supplied private data and operations for dma_buf_ops. On either missing
> * ops, or error in allocating struct dma_buf, will return negative error.
> *
> */
> -struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops
> *ops,
> - size_t size, int flags, const char *exp_name,
> - struct reservation_object *resv)
> +struct dma_buf *dma_buf_export(struct dma_buf_export_info *exp_info)
This function should probably take a const struct dma_buf_export_info here..
Rest looks sane.
~Maarten
> {
> struct dma_buf *dmabuf;
> struct file *file;
> size_t alloc_size = sizeof(struct dma_buf);
> - if (!resv)
> + if (!exp_info->resv)
> alloc_size += sizeof(struct reservation_object);
> else
> /* prevent &dma_buf[1] == dma_buf->resv */
> alloc_size += 1;
>
> - if (WARN_ON(!priv || !ops
> - || !ops->map_dma_buf
> - || !ops->unmap_dma_buf
> - || !ops->release
> - || !ops->kmap_atomic
> - || !ops->kmap
> - || !ops->mmap)) {
> + if (WARN_ON(!exp_info->priv
> + || !exp_info->ops
> + || !exp_info->ops->map_dma_buf
> + || !exp_info->ops->unmap_dma_buf
> + || !exp_info->ops->release
> + || !exp_info->ops->kmap_atomic
> + || !exp_info->ops->kmap
> + || !exp_info->ops->mmap)) {
> return ERR_PTR(-EINVAL);
> }
>
> @@ -309,21 +310,22 @@ struct dma_buf *dma_buf_export_named(void *priv, const
> struct dma_buf_ops *ops,
> if (dmabuf == NULL)
> return ERR_PTR(-ENOMEM);
>
> - dmabuf->priv = priv;
> - dmabuf->ops = ops;
> - dmabuf->size = size;
> - dmabuf->exp_name = exp_name;
> + dmabuf->priv = exp_info->priv;
> + dmabuf->ops = exp_info->ops;
> + dmabuf->size = exp_info->size;
> + dmabuf->exp_name = exp_info->exp_name;
> init_waitqueue_head(&dmabuf->poll);
> dmabuf->cb_excl.poll = dmabuf->cb_shared.poll = &dmabuf->poll;
> dmabuf->cb_excl.active = dmabuf->cb_shared.active = 0;
>
> - if (!resv) {
> - resv = (struct reservation_ob
[Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
Now when we declare gpu errors only through our own dedicated
hangcheck workqueue there is no need to have a separate workqueue
for handling the resetting and waking up the clients as the deadlock
concerns are no more.
The only exception is i915_debugfs::i915_set_wedged, which triggers
error handling through process context. However as this is only used through
test harness it is responsibility for test harness not to introduce hangs
through both debug interface and through hangcheck mechanism at the same time.
Remove gpu_error.work and let the hangcheck work do the tasks it used to.
v2: Add a big warning sign into i915_debugfs::i915_set_wedged (Chris)
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 +++
drivers/gpu/drm/i915/i915_dma.c | 1 -
drivers/gpu/drm/i915/i915_drv.h | 2 --
drivers/gpu/drm/i915/i915_irq.c | 34 +-
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b332a4..211d494 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3969,6 +3969,17 @@ i915_wedged_set(void *data, u64 val)
struct drm_device *dev = data;
struct drm_i915_private *dev_priv = dev->dev_private;
+ /*
+* There is no safeguard against this debugfs entry colliding
+* with the hangcheck calling same i915_handle_error() in
+* parallel, causing an explosion. For now we assume that the
+* test harness is responsible enough not to inject gpu hangs
+* while it is writing to 'i915_wedged'
+*/
+
+ if (i915_reset_in_progress(&dev_priv->gpu_error))
+ return -EAGAIN;
+
intel_runtime_pm_get(dev_priv);
i915_handle_error(dev, val,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 6eaf795..1a46787 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -945,7 +945,6 @@ int i915_driver_unload(struct drm_device *dev)
/* Free error state after interrupts are fully disabled. */
cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
- cancel_work_sync(&dev_priv->gpu_error.work);
i915_destroy_error_state(dev);
if (dev->pdev->msi_enabled)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b09173f..07f99ca 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1352,8 +1352,6 @@ struct i915_gpu_error {
spinlock_t lock;
/* Protected by the above dev->gpu_error.lock. */
struct drm_i915_error_state *first_error;
- struct work_struct work;
-
unsigned long missed_irq_rings;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 234b1f7..44dbf78 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2421,19 +2421,15 @@ static void i915_error_wake_up(struct drm_i915_private
*dev_priv,
}
/**
- * i915_error_work_func - do process context error handling work
- * @work: work struct
+ * i915_reset_and_wakeup - do process context error handling work
*
* Fire an error uevent so userspace can see that a hang or error
* was detected.
*/
-static void i915_error_work_func(struct work_struct *work)
+static void i915_reset_and_wakeup(struct drm_device *dev)
{
- struct i915_gpu_error *error = container_of(work, struct i915_gpu_error,
- work);
- struct drm_i915_private *dev_priv =
- container_of(error, struct drm_i915_private, gpu_error);
- struct drm_device *dev = dev_priv->dev;
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ struct i915_gpu_error *error = &dev_priv->gpu_error;
char *error_event[] = { I915_ERROR_UEVENT "=1", NULL };
char *reset_event[] = { I915_RESET_UEVENT "=1", NULL };
char *reset_done_event[] = { I915_ERROR_UEVENT "=0", NULL };
@@ -2600,10 +2596,10 @@ static void i915_report_and_clear_eir(struct drm_device
*dev)
}
/**
- * i915_handle_error - handle an error interrupt
+ * i915_handle_error - handle a gpu error
* @dev: drm device
*
- * Do some basic checking of regsiter state at error interrupt time and
+ * Do some basic checking of regsiter state at error time and
* dump it to the syslog. Also call i915_capture_error_state() to make
* sure we get a record and make it available in debugfs. Fire a uevent
* so userspace knows something bad happened (should trigger collection
@@ -2616,6 +2612,9 @@ void i915_handle_error(struct drm_device *dev, bool
wedged,
va_list args;
char error_msg[80];
+ if (WARN_ON(mutex_is_locked(&dev_priv->dev->struct_mutex)))
+ return;
+
va_start(args, fmt);
vscnprintf(error_msg, sizeof(error_msg), fm
Re: [Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
On Wed, Jan 28, 2015 at 05:03:14PM +0200, Mika Kuoppala wrote: > Now when we declare gpu errors only through our own dedicated > hangcheck workqueue there is no need to have a separate workqueue > for handling the resetting and waking up the clients as the deadlock > concerns are no more. > > The only exception is i915_debugfs::i915_set_wedged, which triggers > error handling through process context. However as this is only used through > test harness it is responsibility for test harness not to introduce hangs > through both debug interface and through hangcheck mechanism at the same time. > > Remove gpu_error.work and let the hangcheck work do the tasks it used to. > > v2: Add a big warning sign into i915_debugfs::i915_set_wedged (Chris) > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/documentation: Add intel_uncore.c to drm.tmpl
Include intel_uncore.c in template for it to include d documentation for intel_uncore_forcewake_get and *_put. Cc: Daniel Vetter Signed-off-by: Mika Kuoppala --- Documentation/DocBook/drm.tmpl | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 77d0455..03f1985 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3969,6 +3969,7 @@ int num_ioctls; Runtime Power Management !Pdrivers/gpu/drm/i915/intel_runtime_pm.c runtime pm !Idrivers/gpu/drm/i915/intel_runtime_pm.c +!Idrivers/gpu/drm/i915/intel_uncore.c Interrupt Handling -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
Ville Syrjälä writes: > On Wed, Jan 28, 2015 at 03:28:56PM +0200, Mika Kuoppala wrote: >> Chris Wilson writes: >> >> > On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: >> >> commit 05a2fb157e44a53c79133805d30eaada43911941 >> >> Author: Mika Kuoppala >> >> Date: Mon Jan 19 16:20:43 2015 +0200 >> >> >> >> drm/i915: Consolidate forcewake code >> >> >> >> introduced domain handling where each domain has it's own posting >> >> read registers. This changed the forcewake sequence on 'put' side when >> >> there is multiple domains as there would be extra read between the domain >> >> puts. Any posting read should be enough to flush all the changes. >> >> >> >> Do a posting read only once, at the end of the sequence and for >> >> the first domain. Like it was before. >> >> >> >> Cc: Chris Wilson >> >> Signed-off-by: Mika Kuoppala >> > >> > fwiw, I would argue that the posting read in _get() is superfluous as we >> > will serialise the fw with not only the ack, but any subsequent mmio. >> > >> > On the _put() side we do want to flush the write so that the hw can >> > power down as early as possible. So just kill the posting read from _get >> > and otherwise drop the patch. :) >> >> Yes, both put/get patches should be dropped. I posted a patch removing >> the posting read on get side and with your explanations in commit message. >> >> This all starts to make so much sense that some gen is bound to break ;) > > IIRC the posting read from same cache line actually fixed real bugs. So > I'm a bit worried about dropping them. But I suppose it's possible only > the _put side was important for those bugs. I found these: commit 6af2d180f82151cf3d58952e35a4f96e45bc453a Author: Daniel Vetter Date: Thu Jul 26 16:24:50 2012 +0200 drm/i915: fix forcewake related hangs on snb commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd Author: Ben Widawsky Date: Sat Sep 1 22:59:50 2012 -0700 drm/i915: Never read FORCEWAKE https://bugs.freedesktop.org/show_bug.cgi?id=51738 https://bugs.freedesktop.org/show_bug.cgi?id=52424 The snb here seems to survive gem_dummy_reloc_loop and gem_ring_sync_loop in here with the get side posting removed. -Mika > > -- > Ville Syrjälä > Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
> Mainly taking care of some register offsets, otherwise things are similar to
> hsw. Also, programming ddi aux to use hardcoded values for psr data select.
>
> v2: introduce EDP_PSR_AUX_BASE macro (Chris)
> v3: Moving to HW tracking for SKL+ platforms, so activating source psr during
> psr_enabling and then avoiding psr entries and exits for each frontbuffer
> updates.
> v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition
> (Rodrigo)
>
> Signed-off-by: Sonika Jindal
> Reviewed-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/i915_drv.h |3 ++-
> drivers/gpu/drm/i915/i915_reg.h |5 +
> drivers/gpu/drm/i915/intel_frontbuffer.c |7 +--
> drivers/gpu/drm/i915/intel_psr.c | 26 --
> 4 files changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 66f0c60..3d24872 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
> - IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> + IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> + IS_SKYLAKE(dev))
> #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
>IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index a39bb03..a6f06fa 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3748,6 +3748,11 @@ enum punit_power_well {
> #define DP_AUX_CH_CTL_PRECHARGE_TEST (1 << 11)
> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK(0x7ff)
> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT 0
> +#define DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
> +#define DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL (1 << 13)
> +#define DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
> +#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
>
> /*
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 79f6d72..010d550 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object
> *obj,
>
> intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
>
> - intel_psr_invalidate(dev, obj->frontbuffer_bits);
> +
> + if (INTEL_INFO(dev)->gen < 9)
> + intel_psr_invalidate(dev, obj->frontbuffer_bits);
> }
>
> /**
> @@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>
> intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>
> - intel_psr_flush(dev, frontbuffer_bits);
> + if (INTEL_INFO(dev)->gen < 9)
> + intel_psr_flush(dev, frontbuffer_bits);
Again no, not going to take wholesale filtering of the sw invalidate
paths. This needs to be properly tested and pushed down into the psr
specific invalidate/flush functions on a per-function basis.
I've dropped these two hunks and merged the patch.
-Daniel
>
> /*
>* FIXME: Unconditional fbc flushing here is a rather gross hack and
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index dd0e6e0..4867d5a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -142,6 +142,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> uint32_t aux_clock_divider;
> + uint32_t aux_data_reg, aux_ctl_reg;
> int precharge = 0x3;
> bool only_standby = dev_priv->vbt.psr.full_link;
> static const uint8_t aux_msg[] = {
> @@ -168,16 +169,34 @@ static void hsw_psr_enable_sink(struct intel_dp
> *intel_dp)
> drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>
> + aux_data_reg = (INTEL_INFO(dev)->gen >= 9) ?
> + DPA_AUX_CH_DATA1 : EDP_PSR_AUX_DATA1(dev);
> + aux_ctl_reg = (INTEL_INFO(dev)->gen >= 9) ?
> + DPA_AUX_CH_CTL : EDP_PSR_AUX_CTL(dev);
> +
> /* Setup AUX registers */
> for (i = 0; i < sizeof(aux_msg); i += 4)
> - I915_WRITE(EDP_PSR_AUX_DATA1(dev) + i,
> + I915_WR
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do only one posting read on forcewake put sequence
On Wed, Jan 28, 2015 at 05:54:14PM +0200, Mika Kuoppala wrote: > Ville Syrjälä writes: > > > On Wed, Jan 28, 2015 at 03:28:56PM +0200, Mika Kuoppala wrote: > >> Chris Wilson writes: > >> > >> > On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: > >> >> commit 05a2fb157e44a53c79133805d30eaada43911941 > >> >> Author: Mika Kuoppala > >> >> Date: Mon Jan 19 16:20:43 2015 +0200 > >> >> > >> >> drm/i915: Consolidate forcewake code > >> >> > >> >> introduced domain handling where each domain has it's own posting > >> >> read registers. This changed the forcewake sequence on 'put' side when > >> >> there is multiple domains as there would be extra read between the > >> >> domain > >> >> puts. Any posting read should be enough to flush all the changes. > >> >> > >> >> Do a posting read only once, at the end of the sequence and for > >> >> the first domain. Like it was before. > >> >> > >> >> Cc: Chris Wilson > >> >> Signed-off-by: Mika Kuoppala > >> > > >> > fwiw, I would argue that the posting read in _get() is superfluous as we > >> > will serialise the fw with not only the ack, but any subsequent mmio. > >> > > >> > On the _put() side we do want to flush the write so that the hw can > >> > power down as early as possible. So just kill the posting read from _get > >> > and otherwise drop the patch. :) > >> > >> Yes, both put/get patches should be dropped. I posted a patch removing > >> the posting read on get side and with your explanations in commit message. > >> > >> This all starts to make so much sense that some gen is bound to break ;) > > > > IIRC the posting read from same cache line actually fixed real bugs. So > > I'm a bit worried about dropping them. But I suppose it's possible only > > the _put side was important for those bugs. > > I found these: > > commit 6af2d180f82151cf3d58952e35a4f96e45bc453a > Author: Daniel Vetter > Date: Thu Jul 26 16:24:50 2012 +0200 > > drm/i915: fix forcewake related hangs on snb > > commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd > Author: Ben Widawsky > Date: Sat Sep 1 22:59:50 2012 -0700 > > drm/i915: Never read FORCEWAKE > > https://bugs.freedesktop.org/show_bug.cgi?id=51738 > https://bugs.freedesktop.org/show_bug.cgi?id=52424 > > The snb here seems to survive gem_dummy_reloc_loop and > gem_ring_sync_loop in here with the get side posting removed. Note that we kept the once associated with #52424, but judging by my comments in #51738 the posting read is just a band aid anyway as a full mb() itself was not adequate. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v3] drm/i915: Android native sync support
On 01/28/2015 09:29 AM, Daniel Vetter wrote: On Tue, Jan 27, 2015 at 11:29:36AM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin Add Android native sync support with fences exported as file descriptors via the execbuf ioctl (rsvd2 field is used). This is a continuation of Jesse Barnes's previous work, squashed to arrive at the final destination, cleaned up, with some fixes and preliminary light testing. GEM requests are extended with fence structures which are associated with Android sync fences exported to user space via file descriptors. Fences which are waited upon, and while exported to userspace, are referenced and added to the irq_queue so they are signalled when requests are completed. There is no overhead apart from the where fences are not requested. Based on patches by Jesse Barnes: drm/i915: Android sync points for i915 v3 drm/i915: add fences to the request struct drm/i915: sync fence fixes/updates To do: * Extend driver data with context id / ring id (TBD). v2: * Code review comments. (Chris Wilson) * ring->add_request() was a wrong thing to call - rebase on top of John Harrison's (drm/i915: Early alloc request) to ensure correct request is present before creating a fence. * Take a request reference from signalling path as well to ensure request sticks around while fence is on the request completion wait queue. v3: * Use worker to unreference on completion so it can lock the mutex from interrupt context. Signed-off-by: Tvrtko Ursulin Cc: Jesse Barnes Cc: John Harrison btw for merging we need to split the conversion to fences out from the actual userspace interface. And imo we should replace a lot of our existing usage of i915_gem_request with fences within the driver, too. Not tacked on the side like in your patch here. All the recent refactoring have been aiming to match i915 requests to the internal fence interfaces, so we should be pretty much there. Ok I did not realize this. It did not make sense to me to split it out if the only access point to create them is via Android native sync, but from what you are saying fences should be initialized and active for all requests all the time. With the native sync part established on demand. In what respects has the refactoring been aligning requests and fences? We also need this to be able to integrate with the scheduler properly - if that needs to deal both with fences and i915 requests separately it'll be a bit more messy. If it's all just fences the code should be streamlined a lot. Requests will remain as main data structure representing the unit of GPU work? Is so, it sounds logical that fences are associated (or aggregated) with requests. I don't see how scheduler would work with fences and not requests. Regards, Tvrtko P.S. Thanks for pointing out self-tuning busy loads in another thread - it works nicely. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] lib/tests: check that invalid subtest names are rejected
Signed-off-by: Thomas Wood
---
lib/tests/Makefile.sources | 2 ++
lib/tests/igt_invalid_subtest_name.c | 31 +++
2 files changed, 33 insertions(+)
create mode 100644 lib/tests/igt_invalid_subtest_name.c
diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
index 828baa4..ecd73ae 100644
--- a/lib/tests/Makefile.sources
+++ b/lib/tests/Makefile.sources
@@ -7,6 +7,7 @@ check_PROGRAMS = \
igt_simulation \
igt_simple_test_subtests \
igt_timeout \
+ igt_invalid_subtest_name \
$(NULL)
check_SCRIPTS = \
@@ -26,4 +27,5 @@ XFAIL_TESTS = \
igt_no_subtest \
igt_simple_test_subtests \
igt_timeout \
+ igt_invalid_subtest_name \
$(NULL)
diff --git a/lib/tests/igt_invalid_subtest_name.c
b/lib/tests/igt_invalid_subtest_name.c
new file mode 100644
index 000..418071d
--- /dev/null
+++ b/lib/tests/igt_invalid_subtest_name.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt_core.h"
+
+igt_main
+{
+ igt_subtest("# invalid name !") {
+ igt_info("Invalid subtest name test\n");
+ }
+}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2] lib/tests: verify subtest enumeration output
Check that the subtest list is not empty if using --list-subtests returns with an exit code of 0, and that the list is empty if it returns with 79. Signed-off-by: Thomas Wood --- lib/tests/igt_command_line.sh | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/tests/igt_command_line.sh b/lib/tests/igt_command_line.sh index 5cf2584..a057943 100755 --- a/lib/tests/igt_command_line.sh +++ b/lib/tests/igt_command_line.sh @@ -56,8 +56,17 @@ for test in $TESTLIST; do # check --list-subtests works correctly echo " Checking subtest enumeration..." - ./$test --list-subtests > /dev/null - if [ $? -ne 0 -a $? -ne 79 ]; then + LIST=`./$test --list-subtests` + RET=$? + if [ $RET -ne 0 -a $RET -ne 79 ]; then + exit 1 + fi + + if [ $RET -eq 79 -a -n "$LIST" ]; then + exit 1 + fi + + if [ $RET -eq 0 -a -z "$LIST" ]; then exit 1 fi -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
From: Rob Clark
In DRM/KMS we are lacking a good way to deal with tiled/compressed
formats. Especially in the case of dmabuf/prime buffer sharing, where
we cannot always rely on under-the-hood flags passed to driver specific
gem-create ioctl to pass around these extra flags.
The proposal is to add a per-plane format modifier. This allows to, if
necessary, use different tiling patters for sub-sampled planes, etc.
The format modifiers are added at the end of the ioctl struct, so for
legacy userspace it will be zero padded.
TODO how best to deal with assignment of modifier token values? The
rough idea was to namespace things with an 8bit vendor-id, and then
beyond that it is treated as an opaque value. But that was a relatively
arbitrary choice. There are cases where same tiling pattern and/or
compression is supported by various different vendors. So we should
standardize to use the vendor-id and value of the first one who
documents the format?
v1: original
v1.5: increase modifier to 64b
v2: Incorporate review comments from the big thread, plus a few more.
- Add a getcap so that userspace doesn't have to jump through hoops.
- Allow modifiers only when a flag is set. That way drivers know when
they're dealing with old userspace and need to fish out e.g. tiling
from other information.
- After rolling out checks for ->modifier to all drivers I've decided
that this is way too fragile and needs an explicit opt-in flag. So
do that instead.
- Add a define (just for documentation really) for the "NONE"
modifier. Imo we don't need to add mask #defines since drivers
really should only do exact matches against values defined with
fourcc_mod_code.
- Drop the Samsung tiling modifier on Rob's request since he's not yet
sure whether that one is accurate.
Cc: Rob Clark
Cc: Tvrtko Ursulin
Cc: Laurent Pinchart
Cc: Daniel Stone
Cc: Ville Syrjälä
Cc: Michel Dänzer
Signed-off-by: Rob Clark (v1.5)
Signed-off-by: Daniel Vetter
--
I've picked this up since we want to push out some fancy new tiling
modes soonish. No defines yet, but Tvrkto is working on the i915 parts
of this.
I think the only part I haven't done from the discussion is exposing a
list of supported modifiers. Not sure that's really useful though
since all this is highly hw specific.
And a note to driver writes: They need to check or the flag and in its
absence make a reasonable choice about the internal layet (e.g. for
i915 consult the tiling mode of the underlying bo).
-Daniel
---
drivers/gpu/drm/drm_crtc.c| 14 +-
drivers/gpu/drm/drm_ioctl.c | 3 +++
include/drm/drm_crtc.h| 3 +++
include/uapi/drm/drm.h| 1 +
include/uapi/drm/drm_fourcc.h | 26 ++
include/uapi/drm/drm_mode.h | 9 +
6 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 419f9d915c78..8090e3d64f67 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3324,6 +3324,12 @@ static int framebuffer_check(const struct
drm_mode_fb_cmd2 *r)
DRM_DEBUG_KMS("bad pitch %u for plane %d\n",
r->pitches[i], i);
return -EINVAL;
}
+
+ if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {
+ DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
+ r->modifier[i], i);
+ return -EINVAL;
+ }
}
return 0;
@@ -3337,7 +3343,7 @@ static struct drm_framebuffer
*add_framebuffer_internal(struct drm_device *dev,
struct drm_framebuffer *fb;
int ret;
- if (r->flags & ~DRM_MODE_FB_INTERLACED) {
+ if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
return ERR_PTR(-EINVAL);
}
@@ -3353,6 +3359,12 @@ static struct drm_framebuffer
*add_framebuffer_internal(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
+ if (r->flags & DRM_MODE_FB_MODIFIERS &&
+ !dev->mode_config.allow_fb_modifiers) {
+ DRM_DEBUG_KMS("driver does not support fb modifiers\n");
+ return ERR_PTR(-EINVAL);
+ }
+
ret = framebuffer_check(r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 3785d66721f2..a6d773a61c2d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -321,6 +321,9 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
else
req->value = 64;
break;
+ case DRM_CAP_ADDFB2_MODIFIERS:
+ req->value = dev->mode_config.allow_fb_modifiers;
+ break;
default:
return -EINVAL;
}
diff --git a/incl
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On 01/28/2015 05:37 PM, Daniel Vetter wrote: From: Rob Clark In DRM/KMS we are lacking a good way to deal with tiled/compressed formats. Especially in the case of dmabuf/prime buffer sharing, where we cannot always rely on under-the-hood flags passed to driver specific gem-create ioctl to pass around these extra flags. The proposal is to add a per-plane format modifier. This allows to, if necessary, use different tiling patters for sub-sampled planes, etc. The format modifiers are added at the end of the ioctl struct, so for legacy userspace it will be zero padded. TODO how best to deal with assignment of modifier token values? The rough idea was to namespace things with an 8bit vendor-id, and then beyond that it is treated as an opaque value. But that was a relatively arbitrary choice. There are cases where same tiling pattern and/or compression is supported by various different vendors. So we should standardize to use the vendor-id and value of the first one who documents the format? Maybe: __u64 modifier[4]; __u64 vendor_modifier[4]; ? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Wed, Jan 28, 2015 at 12:37 PM, Daniel Vetter wrote:
> From: Rob Clark
>
> In DRM/KMS we are lacking a good way to deal with tiled/compressed
> formats. Especially in the case of dmabuf/prime buffer sharing, where
> we cannot always rely on under-the-hood flags passed to driver specific
> gem-create ioctl to pass around these extra flags.
>
> The proposal is to add a per-plane format modifier. This allows to, if
> necessary, use different tiling patters for sub-sampled planes, etc.
> The format modifiers are added at the end of the ioctl struct, so for
> legacy userspace it will be zero padded.
>
> TODO how best to deal with assignment of modifier token values? The
> rough idea was to namespace things with an 8bit vendor-id, and then
> beyond that it is treated as an opaque value. But that was a relatively
> arbitrary choice. There are cases where same tiling pattern and/or
> compression is supported by various different vendors. So we should
> standardize to use the vendor-id and value of the first one who
> documents the format?
iirc, I think we decided that drm_fourcc.h in drm-next is a sufficient
authority for coordinating assignment of modifier token values, so we
could probably drop this TODO. Perhaps it wouldn't hurt to document
somewhere that, as with fourcc/format values, new additions are
expected to come with some description of the format?
>
> v1: original
> v1.5: increase modifier to 64b
>
> v2: Incorporate review comments from the big thread, plus a few more.
>
> - Add a getcap so that userspace doesn't have to jump through hoops.
> - Allow modifiers only when a flag is set. That way drivers know when
> they're dealing with old userspace and need to fish out e.g. tiling
> from other information.
> - After rolling out checks for ->modifier to all drivers I've decided
> that this is way too fragile and needs an explicit opt-in flag. So
> do that instead.
> - Add a define (just for documentation really) for the "NONE"
> modifier. Imo we don't need to add mask #defines since drivers
> really should only do exact matches against values defined with
> fourcc_mod_code.
> - Drop the Samsung tiling modifier on Rob's request since he's not yet
> sure whether that one is accurate.
>
> Cc: Rob Clark
> Cc: Tvrtko Ursulin
> Cc: Laurent Pinchart
> Cc: Daniel Stone
> Cc: Ville Syrjälä
> Cc: Michel Dänzer
> Signed-off-by: Rob Clark (v1.5)
> Signed-off-by: Daniel Vetter
>
you forgot to change subject line to (v2).. but other than that, looks good
Reviewed-by: Rob Clark
> --
>
> I've picked this up since we want to push out some fancy new tiling
> modes soonish. No defines yet, but Tvrkto is working on the i915 parts
> of this.
>
> I think the only part I haven't done from the discussion is exposing a
> list of supported modifiers. Not sure that's really useful though
> since all this is highly hw specific.
>
> And a note to driver writes: They need to check or the flag and in its
> absence make a reasonable choice about the internal layet (e.g. for
> i915 consult the tiling mode of the underlying bo).
> -Daniel
> ---
> drivers/gpu/drm/drm_crtc.c| 14 +-
> drivers/gpu/drm/drm_ioctl.c | 3 +++
> include/drm/drm_crtc.h| 3 +++
> include/uapi/drm/drm.h| 1 +
> include/uapi/drm/drm_fourcc.h | 26 ++
> include/uapi/drm/drm_mode.h | 9 +
> 6 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 419f9d915c78..8090e3d64f67 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3324,6 +3324,12 @@ static int framebuffer_check(const struct
> drm_mode_fb_cmd2 *r)
> DRM_DEBUG_KMS("bad pitch %u for plane %d\n",
> r->pitches[i], i);
> return -EINVAL;
> }
> +
> + if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {
> + DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
> + r->modifier[i], i);
> + return -EINVAL;
> + }
> }
>
> return 0;
> @@ -3337,7 +3343,7 @@ static struct drm_framebuffer
> *add_framebuffer_internal(struct drm_device *dev,
> struct drm_framebuffer *fb;
> int ret;
>
> - if (r->flags & ~DRM_MODE_FB_INTERLACED) {
> + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
> DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
> return ERR_PTR(-EINVAL);
> }
> @@ -3353,6 +3359,12 @@ static struct drm_framebuffer
> *add_framebuffer_internal(struct drm_device *dev,
> return ERR_PTR(-EINVAL);
> }
>
> + if (r->flags & DRM_MODE_FB_MODIFIERS &&
> + !dev->mode_config.allow_fb_modifiers) {
> + DRM_DEBUG_KMS("driver does not support fb modifiers\n");
> + return E
Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
Mainly taking care of some register offsets, otherwise things are similar to
hsw. Also, programming ddi aux to use hardcoded values for psr data select.
v2: introduce EDP_PSR_AUX_BASE macro (Chris)
v3: Moving to HW tracking for SKL+ platforms, so activating source psr during
psr_enabling and then avoiding psr entries and exits for each frontbuffer
updates.
v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition (Rodrigo)
Signed-off-by: Sonika Jindal
Reviewed-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_drv.h |3 ++-
drivers/gpu/drm/i915/i915_reg.h |5 +
drivers/gpu/drm/i915/intel_frontbuffer.c |7 +--
drivers/gpu/drm/i915/intel_psr.c | 26 --
4 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 66f0c60..3d24872 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
#define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \
-IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
+IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
+IS_SKYLAKE(dev))
#define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
#define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a39bb03..a6f06fa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3748,6 +3748,11 @@ enum punit_power_well {
#define DP_AUX_CH_CTL_PRECHARGE_TEST(1 << 11)
#define DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK(0x7ff)
#define DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT 0
+#define DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
+#define DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL(1 << 13)
+#define DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
+#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
+#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
#define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
/*
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 79f6d72..010d550 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object
*obj,
intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
- intel_psr_invalidate(dev, obj->frontbuffer_bits);
+
+ if (INTEL_INFO(dev)->gen < 9)
+ intel_psr_invalidate(dev, obj->frontbuffer_bits);
}
/**
@@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
- intel_psr_flush(dev, frontbuffer_bits);
+ if (INTEL_INFO(dev)->gen < 9)
+ intel_psr_flush(dev, frontbuffer_bits);
Again no, not going to take wholesale filtering of the sw invalidate
paths. This needs to be properly tested and pushed down into the psr
specific invalidate/flush functions on a per-function basis.
I've dropped these two hunks and merged the patch.
-Daniel
Hi Daniel,
Even SW tracking doesn't work in many cases, like I reported earlier in
ubuntu login screen where we don't get frontbuffer flushes and we don't
enter PSR at all with SW tracking.
I see similar behavior even in fbcon mode. So, I am not sure how you can
say that SW tracking is the only right way.
If there are cases where HW tracking fails (and I know a few), we need
to fix them. I can move this gen check to the intel_psr_* function if
that is the major concern.
-Sonika
/*
* FIXME: Unconditional fbc flushing here is a rather gross hack and
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index dd0e6e0..4867d5a 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -142,6 +142,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t aux_clock_divider;
+ uint32_t aux_data_reg, aux_ctl_reg;
int precharge = 0x3;
bool only_standby = dev_priv->vbt.psr.full_link;
static const uint8_t aux_msg[] = {
@@ -168,16 +169,34 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIV
Re: [Intel-gfx] [PATCH v2] drm/i915/dsi: switch to drm_panel interface
On 01/23/2015 07:00 PM, Jani Nikula wrote:
Replace intel_dsi_device and intel_dsi_dev_ops with drm_panel and
drm_panel_funcs. They are adequate for what we have now, and if we end
up needing more than this we should improve drm_panel. This will keep us
better aligned with the drm core infrastructure.
The panel driver initialization changes a bit. It still remains hideous,
but fixing that is beyond the scope here.
v2: extend mode config mutex to cover drm_panel_get_modes (Shobhit)
vbt_panel->intel_dsi = intel_dsi in vbt panel init (Shobhit)
Signed-off-by: Jani Nikula
Reviewed-By: Shobhit Kumar
---
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/intel_dsi.c | 68 +++
drivers/gpu/drm/i915/intel_dsi.h | 27 +
drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 180 ++---
4 files changed, 157 insertions(+), 119 deletions(-)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4e39ab34eb1c..da196cd07263 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -11,6 +11,7 @@ config DRM_I915
select SHMEM
select TMPFS
select DRM_KMS_HELPER
+ select DRM_PANEL
# i915 depends on ACPI_VIDEO when ACPI is enabled
# but for select to work, need to select ACPI_VIDEO's dependencies, ick
select BACKLIGHT_LCD_SUPPORT if ACPI
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c56c4150fc13..52c5fabe284a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -27,18 +27,20 @@
#include
#include
#include
+#include
#include
#include "i915_drv.h"
#include "intel_drv.h"
#include "intel_dsi.h"
#include "intel_dsi_cmd.h"
-/* the sub-encoders aka panel drivers */
-static const struct intel_dsi_device intel_dsi_devices[] = {
+static const struct {
+ u16 panel_id;
+ struct drm_panel * (*init)(struct intel_dsi *intel_dsi, u16 panel_id);
+} intel_dsi_drivers[] = {
{
.panel_id = MIPI_DSI_GENERIC_PANEL_ID,
- .name = "vbt-generic-dsi-vid-mode-display",
- .dev_ops = &vbt_generic_dsi_display_ops,
+ .init = vbt_panel_init,
},
};
@@ -214,8 +216,7 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
dpi_send_cmd(intel_dsi, TURN_ON, DPI_LP_MODE_EN, port);
msleep(100);
- if (intel_dsi->dev.dev_ops->enable)
- intel_dsi->dev.dev_ops->enable(&intel_dsi->dev);
+ drm_panel_enable(intel_dsi->panel);
for_each_dsi_port(port, intel_dsi->ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -255,8 +256,7 @@ static void intel_dsi_pre_enable(struct intel_encoder
*encoder)
msleep(intel_dsi->panel_on_delay);
- if (intel_dsi->dev.dev_ops->panel_reset)
- intel_dsi->dev.dev_ops->panel_reset(&intel_dsi->dev);
+ drm_panel_prepare(intel_dsi->panel);
for_each_dsi_port(port, intel_dsi->ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -329,8 +329,7 @@ static void intel_dsi_disable(struct intel_encoder *encoder)
}
/* if disable packets are sent before sending shutdown packet then in
* some next enable sequence send turn on packet error is observed */
- if (intel_dsi->dev.dev_ops->disable)
- intel_dsi->dev.dev_ops->disable(&intel_dsi->dev);
+ drm_panel_disable(intel_dsi->panel);
for_each_dsi_port(port, intel_dsi->ports)
wait_for_dsi_fifo_empty(intel_dsi, port);
@@ -395,8 +394,7 @@ static void intel_dsi_post_disable(struct intel_encoder
*encoder)
val &= ~DPOUNIT_CLOCK_GATE_DISABLE;
I915_WRITE(DSPCLK_GATE_D, val);
- if (intel_dsi->dev.dev_ops->disable_panel_power)
- intel_dsi->dev.dev_ops->disable_panel_power(&intel_dsi->dev);
+ drm_panel_unprepare(intel_dsi->panel);
msleep(intel_dsi->panel_off_delay);
msleep(intel_dsi->panel_pwr_cycle_delay);
@@ -760,7 +758,7 @@ static int intel_dsi_get_modes(struct drm_connector
*connector)
return 1;
}
-static void intel_dsi_destroy(struct drm_connector *connector)
+static void intel_dsi_connector_destroy(struct drm_connector *connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
@@ -770,8 +768,20 @@ static void intel_dsi_destroy(struct drm_connector
*connector)
kfree(connector);
}
+static void intel_dsi_encoder_destroy(struct drm_encoder *encoder)
+{
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
+
+ if (intel_dsi->panel) {
+ drm_panel_detach(intel_dsi->panel);
+ /* XXX: Logically this call belongs in the panel driver. */
+ drm_panel_remove(intel_dsi->panel);
+ }
+ intel_encoder_destroy(en
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
> > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
> > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
> > > this WARN at boot (and pretty frequently afterwards):
> > >
> > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377
> > > gen6_set_rps+0x371/0x3c0()
> > > WARN_ON(val > dev_priv->rps.max_freq_softlimit)
> >
> > [snip]
> >
> > > I'm not at all familiar with this hardware, but I took a quick look into
> > > what changed with this commit for my laptop. Before the commit,
> > > rps.min_freq_softlimit is 4 (from rps.min_freq) and
> > > rps.max_freq_softlimit is 22.
> > >
> > > After the commit, rps.min_freq_softlimit is set to the
> > > rps.efficient_freq value read from pcode, which is 34 on my laptop.
> > > So later when gen6_set_rps() is called with rps.min_freq_softlimit that
> > > warning is hit.
> > >
> > > Any thoughts? It certainly seems fishy that this commit causes
> > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
> >
> > Very fishy indeed. Moral of this story, never trust hw.
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 3e630feb18e4..bbedd2901c54 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct
> > drm_device *dev)
> > &ddcc_status);
> > if (0 == ret)
> > dev_priv->rps.efficient_freq =
> > - (ddcc_status >> 8) & 0xff;
> > + clamp_t(u8,
> > + (ddcc_status >> 8) & 0xff,
> > + dev_priv->rps.min_freq,
> > + dev_priv->rps.max_freq);
>
> Maybe better to fall back to rp1_freq if this is bogus?
>
[TOR:] Michael, Thank you for bringing this problem to our attention.
Yes, this function needs some range checking to maintain
RPn <= RPe <= RP0.
A value of 34 seems too high for RPe.
What values does the Carbon X1 (Haswell) have for RPn and RP0?
I agree with Ville that a bogus value from the pcode read should
not be used. Simple clamping would set the min_freq to RP0;
probably incorrect.
Tom O'Rourke
> > }
> >
> > /* Preserve min/max settings in case of re-init */
> >
> > But really it is probably just best to disable the query for hsw:
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 3e630feb18e4..01bd508e81f6 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct
> > drm_device *dev)
> > dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
> >
> > dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> > - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > + if (IS_BROADWELL(dev)) {
> > ret = sandybridge_pcode_read(dev_priv,
> >
> > HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> > &ddcc_status);
> >
> > Paranoia says we do both.
> > -Chris
> >
> > --
> > Chris Wilson, Intel Open Source Technology Centre
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
