- 20 2月, 2016 1 次提交
-
-
由 Paulo Zanoni 提交于
These platforms should be fine now. FBC can allow very significant power savings for screen-on idle systems, but it is worth mentioning that a lot of people won't get significant power savings by enabling this feature because they may have something else preventing the system from getting into the deepest sleep states. Examples may include a hungry wifi device or a max_performance SATA link power management policy. You can check your PC state residencies on the powertop "Idle stats" tab. I recommend trying to run "sudo powertop --auto-tune" and then seeing if the residencies improve. Oh, and in case you - the person reading this commit message - found this commit through git bisect, please do the following: - Check your dmesg and see if there are error messages mentioning underruns around the time your problem started happening. - Download intel-gpu-tools, compile it, and run: $ sudo ./tests/kms_frontbuffer_tracking --run-subtest '*fbc-*' 2>&1 | tee fbc.txt Then send us the fbc.txt file, especially if you get a failure. This will really maximize your chances of getting the bug fixed quickly. - Try to find a reliable way to reproduce the problem, and tell us. - Boot with drm.debug=0xe, reproduce the problem, then send us the dmesg file. v2: Don't enable by default on SKL. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/1455655643-2535-1-git-send-email-paulo.r.zanoni@intel.com
-
- 05 2月, 2016 2 次提交
-
-
由 Paulo Zanoni 提交于
Now that we have top-level gen-independent hw_activate and hw_deactivate functions, set fbc->active directly from them, removing the duplicated code. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1454101060-23198-3-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
The recent introduction of a new caller of dev_priv->fbc.deactivate() is a good example of why we need unexport those functions. Anything outside intel_fbc.c should only call the functions exported by intel_fbc.c, so in order to enforce that, kill the function pointers stored inside dev_priv->fbc and replace them with functions that can't be called from outside intel_fbc.c. This should make it much harder for new code to call these functions from outside intel_fbc.c. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1454101060-23198-2-git-send-email-paulo.r.zanoni@intel.com
-
- 30 1月, 2016 22 次提交
-
-
由 Paulo Zanoni 提交于
The FBC fixes we've been doing in the last months required a lot of refactor, so functions that were once big and called from different spots are now small and called only once. IMHO now it's better to just move the contents of these functions to their only callers since this reduces the number of indirections while reading the code. While at it, also improve the related comments a little bit. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-26-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
We already make sure we run intel_fbc_update_update during modesets and page flips, and this function takes care of deactivating FBC, so it shouldn't be possible for us to reach the condition we check at intel_fbc_work_fn. So instead of grabbing framebuffer references and adding a lot of code to track when we need to free them, just don't track anything at all since we shouldn't need to. v2: Rebase. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-25-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
We don't actually use fb_id anywhere. We already compare all parameters that matter to the hardware: pixel format, stride, fence_reg and ggtt_offset. The ID shouldn't make a difference. Besides, we already update the FBC data at every modeset/flip, so this can't change behind our backs. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-23-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Our dmesg messages started being misleading after we converted to the enable+activate model: we always print "Disabling FBC", even when we're just deactivating it. So, for example, when I boot my machine and do "dmesg | grep -i fbc", I see: [drm:intel_fbc_enable] Enabling FBC on pipe A [drm:set_no_fbc_reason] Disabling FBC: framebuffer not tiled or fenced but then, if I read the debugfs file, I will see: $ sudo cat i915_fbc_status FBC enabled Compressing: yes so we can conclude that dmesg is misleading, since FBC is actually enabled. What happened is that we deactivated FBC due to fbcon not being tiled, but when we silently reactivated it when the display manager started. We don't print activation messages since there may be way too many of these operations per second during normal desktop usage. One possible solution would be to change set_no_fbc_reason to correctly differentiate between disable and deactivation, but we removed support from printing activation/deactivation messages in the past because they were too frequent. So instead of doing this, let's just not print anything on dmesg, and leave the debugfs file if the user needs to investigate something. We already print when we enable and disable FBC anyway on a given pipe, so this should already help triaging bugs. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-22-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
During FBC invalidation, don't call intel_fbc_deactivate if it's not enabled. This doesn't fix any bug, but helps making the interface saner. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-21-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Move intel_fbc_enable to a place where it is called regardless of the "modeset" variable, and make sure intel_fbc_enable can be called multiple times without intel_fbc_disable being called. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-20-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Instead of duplicating the calls for every platform, let's just put them in the correct places inside intel_atomic_commit. This will also make it easier for us to move the enable call in order to support fasbtoot. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-19-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
This opens the possibility of implementing nicer schemes to choose the CRTC, such as checking the amount of stolen memory available, or choosing the best pipe on platforms that don't die FBC to pipe or plane A. This code was written for another refactor that I ended up discarding, so I don't actually need it, but I figured this patch would be an improvement on its own so I kept it on the series. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-18-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Older FBC platforms have this restriction where FBC can't be enabled if multiple pipes are enabled. In the current code, we disable FBC before the second pipe becomes visible. One of the problems with this code is that the current multiple_pipes_ok() implementation just iterates through all CRTCs looking at their states, but it doesn't make sure that the state locks are grabbed. It also can't just grab the locks for every CRTC since this would kill one of the biggest advantages of atomic modesetting. After the recent FBC changes, we now have the appropriate locks for the given CRTC, so we can just try to maintain the state of each CRTC and update it once intel_fbc_pre_update is called. As a last note, I don't have gen 2/3 machines to test this code. My current plan is to enable FBC on just the newer platforms, so this patch is just an attempt to get the gen 2/3 code at least looking sane, so if one day someone decide to fix FBC on these platforms, they may have less work to do. Not-tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> (only on HSW+) Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-16-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Just to be sure nothing will survive a module unload. We need to do this after the unlock in order to make sure the function won't get stuck trying to grab the lock we already own while we wait for it to finish. Reported-by: NReported-by: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-15-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Instead of: - intel_fbc_disable_crtc(crtc) - intel_fbc_disable(dev_priv) we now have: - intel_fbc_disable(crtc) - intel_fbc_global_disable(dev_priv) This is because all the other functions that take a CRTC are called - intel_fbc_something(crtc) Instead of: - intel_fbc_something_crtc(crtc) And I also hope that the word "global" is going to help make it more explicit that "global" is the unusual case, not the opposite. Reported-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-14-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
With the addition and usage of intel_fbc_pre_update, intel_fbc_deactivate is not used anymore outside intel_fbc.c, so kill the exported function and rename __intel_fbc_deactivate. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-13-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
We'll now call intel_fbc_pre_update instead of intel_fbc_deactivate during atomic commits. This will continue to guarantee that we deactivate FBC and it will also update the state checking structures at the correct time. Then, later, at the point where we were calling intel_fbc_update, we'll only need to call intel_fbc_post_update. Also add the proper warnings in case we don't have the appropriate locks. Daniel mentioned the warnings will have to be removed for async commits, but let's keep them here while we can. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-12-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
So now pre_update will be responsible for unconditionally deactivating FBC and updating the state cache, while post_update will be responsible for checking if it can be enabled, then enabling it. This is one more step into proper locking. Notice that intel_fbc_flush now calls post_update directly. The FBC flush can only happen for drawing operations - since we explicitly ignore the flips -, so the FBC state is not expected to have changed at this point. With this we can just run post_update, which will make sure we won't deactivate+reactivate FBC as would be the case now if we called pre_update + post_update. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-11-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Per the new atomic locking rules, we need to cache the CRTC, plane and FB state structures we use so we can access them later without needing more locks. So do this. Notice that there are some pieces of the FBC code that look at things that are only computed during the modeset, so we can't just can't precompute whether FBC can be activated during the update_state_cache stage. We may be able to do this later. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-10-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
If frontbuffer_bits doesn't match the current frontbuffer, there's no reason to recompress or update FBC. There was a plan to make the FBC test suite catch this type of problem, but it never got implemented due to being low priority. While at it, also implement Ville's suggestion and use plane->frontbuffer_bit instead of INTEL_FRONTBUFFER_PRIMARY. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-8-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Before this patch, page flips would call intel_frontbuffer_flip() and intel_frontbuffer_flip_complete(), which would call intel_fbc_flush(), which would call intel_fbc_update(). The problem is that drawing operations also trigger intel_fbc_flush() calls, so it's not guaranteed that we have the CRTC and FB locks grabbed when intel_fbc_flush() happens, since the call trace may come from the rendering path. We're trying to make the FBC code grab the appropriate CRTC/FB locks, so split the drawing and the flipping logic in order to achieve that in later patches. So now the frontbuffer tracking code is just going to be used for frontbuffer drawing, and intel_fbc_update() is going to be used directly for actual page flips. As a note, we don't need to call intel_fbc_flip() during the two places where we call intel_frontbuffer_flip() since in one of them we already have an intel_fbc_update() call, and in the other we have the planes disabled. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-7-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
We say "dev_priv->fbc.something" way too many times in our code while we could be saying just "fbc->something" with a previous declaration of fbc. This has been bothering me for a while but I didn't want to patch it since I wanted to fix the real problems first. But as I add more code I keep thinking about it, especially since it makes the code easier to read and it can make us fit 80 columns easier, so let's just do the change now. While at it, also rename from i915_fbc to intel_fbc because the whole FBC code uses intel_fbc. v2: Rebase after the work_fn changes. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453406763-10400-1-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
The early return inside __intel_fbc_update does not completely check all the parameters that affect the FBC register values. For example, we currently lack looking at crtc->adjusted_y (for the fence Y offset) and all the parameters that affect the CFB size (for i8xx). Instead of just adding the missing parameters to the check and hoping that any changes to the fbc_activate functions also come with a matching change to the __intel_fbc_update check, introduce a new structure where we store these parameters and use the structure at the fbc_activate function. Of course, it's still possible to access everything from dev_priv in those functions, but IMHO the new code will be harder to break. v2: Rebase. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-5-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Make our enable/activate checking model more explicit, especially since we now have intel_fbc_can_activate(). Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-4-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Extract all the code that checks if the FBC configuration is valid to its own function, making __intel_fbc_update() much simpler. Reviewed-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453210558-7875-3-git-send-email-paulo.r.zanoni@intel.com
-
由 Paulo Zanoni 提交于
Instead of waiting for 50ms, just wait until the next vblank, since it's the minimum requirement. The whole infrastructure of FBC is based on vblanks, so waiting for X vblanks instead of X milliseconds sounds like the correct way to go. Besides, 50ms may be less than a vblank on super slow modes that may or may not exist. There are some small improvements in PC state residency (due to the fact that we're now using 16ms for the common modes instead of 50ms), but the biggest advantage is still the correctness of being vblank-based instead of time-based. v2: - Rebase after changing the patch order. - Update the commit message. v3: - Fix bogus vblank_get() instead of vblank_count() (Ville). - Don't forget to call drm_crtc_vblank_{get,put} (Chris, Ville) - Adjust the performance details on the commit message. v4: - Don't grab the FBC mutex just to grab the vblank (Maarten) Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NRodrigo Vivi <rodrigo.vivi@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1453406585-10233-1-git-send-email-paulo.r.zanoni@intel.com
-
- 03 12月, 2015 11 次提交
-
-
由 Paulo Zanoni 提交于
There's no need to stop and restart FBC, which is quite expensive as we have to revalidate the CRTC state. After flushing a drawing operation we know the CRTC state hasn't changed, so a nuke (recompress) should be fine. v2: Make it simpler (Chris). v3: Rewrite the patch again due to patch order changes. v4: Rewrite commit message (Chris). Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
When running Cinnamon I see way too many pairs of these messages: many per second. Get rid of them as they're just telling us FBC is working as expected. We already have the messages for enable/disable, so we don't really need messages for activation/deactivation. v2: Rebase after changing the patch order. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
Directly call intel_fbc_calculate_cfb_size() in the only place that actually needs it, and use the proper check before removing the stolen node. IMHO, this change makes our code easier to understand. v2: Use drm_mm_node_allocated() (Chris). Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
This was already on my TODO list, and was requested both by Chris and Ville, for different reasons. The advantages are avoiding a frequent malloc/free pair, and the locality of having the work structure embedded in dev_priv. The maximum used memory is also smaller since previously we could have multiple allocated intel_fbc_work structs at the same time, and now we'll always have a single one - the one embedded on dev_priv. Of course, we're now using a little more memory on the cases where there's nothing scheduled. The biggest challenge here is to keep everything synchronized the way it was before. Currently, when we try to activate FBC, we allocate a new intel_fbc_work structure. Then later when we conclude we must delay the FBC activation a little more, we allocate a new intel_fbc_work struct, and then adjust dev_priv->fbc.fbc_work to point to the new struct. So when the old work runs - at intel_fbc_work_fn() - it will check that dev_priv->fbc.fbc_work points to something else, so it does nothing. Everything is also protected by fbc.lock. Just cancelling the old delayed work doesn't work because we might just cancel it after the work function already started to run, but while it is still waiting to grab fbc.lock. That's why we use the "dev_priv->fbc.fbc_work == work" check described in the paragraph above. So now that we have a single work struct we have to introduce a new way to synchronize everything. So we're making the work function a normal work instead of a delayed work, and it will be responsible for sleeping the appropriate amount of time itself. This way, after it wakes up it can grab the lock, ask "were we delayed or cancelled?" and then go back to sleep, enable FBC or give up. v2: - Spelling fixes. - Rebase after changing the patch order. - Fix ms/jiffies confusion. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1) Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
This moves the pre-gen4 check from update() to enable(). The HAS_DDI in the original code is not needed since only gen 2/3 have the plane swapping code. v2: Rebase. v3: Extract fbc_on_plane_a_only() (Chris). Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
One of the problems with the current code is that it frees the CFB and releases its drm_mm node as soon as we flip FBC's enable bit. This is bad because after we disable FBC the hardware may still use the CFB for the rest of the frame, so in theory we should only release the drm_mm node one frame after we disable FBC. Otherwise, a stolen memory allocation done right after an FBC disable may result in either corrupted memory for the new owner of that memory region or corrupted screen/underruns in case the new owner changes it while the hardware is still reading it. This case is not exactly easy to reproduce since we currently don't do a lot of stolen memory allocations, but I see patches on the mailing list trying to expose stolen memory to user space, so races will be possible. I thought about three different approaches to solve this, and they all have downsides. The first approach would be to simply use multiple drm_mm nodes and freeing the unused ones only after a frame has passed. The problem with this approach is that since stolen memory is rather small, there's a risk we just won't be able to allocate a new CFB from stolen if the previous one was not freed yet. This could happen in case we quickly disable FBC from pipe A and decide to enable it on pipe B, or just if we change pipe A's fb stride while FBC is enabled. The second approach would be similar to the first one, but maintaining a single drm_mm node and keeping track of when it can be reused. This would remove the disadvantage of not having enough space for two nodes, but would create the new problem where we may not be able to enable FBC at the point intel_fbc_update() is called, so we would have to add more code to retry updating FBC after the time has passed. And that can quickly get too complex since we can get invalidate, flush, disable and other calls in the middle of the wait. Both solutions above - and also the current code - have the problem that we unnecessarily free+realloc FBC during invalidate+flush operations even if the CFB size doesn't change. The third option would be to move the allocation/deallocation to enable/disable. This makes sure that the pipe is always disabled when we allocate/deallocate the CFB, so there's no risk that the FBC hardware may read or write to the memory right after it is freed from drm_mm. The downside is that it is possible for user space to change the buffer stride without triggering a disable/enable - only deactivate/activate -, so we'll have to handle this case somehow - see igt's kms_frontbuffer_tracking test, fbc-stridechange subtest. It could be possible to implement a way to free+alloc the CFB during said stride change, but it would involve a lot of book-keeping - exactly as mentioned above - just for on case, so for now I'll keep it simple and just deactivate FBC. Besides, we may not even need to disable FBC since we do CFB over-allocation. Note from Chris: "Starting a fullscreen client that covers a single monitor in a multi-monitor setup will trigger a change in stride on one of the CRTCs (the monitors will be flipped independently).". It shouldn't be a huge problem if we lose FBC on multi-monitor setups since these setups already have problems reaching deep PC states anyway. v2: Rebase after changing the patch order. v3: - Remove references to the stride change case being "uncommon" and paste Chris' example. - Rebase after a change in a previous patch. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
The goal is to call FBC enable/disable only once per modeset, while activate/deactivate/update will be called multiple times. The enable() function will be responsible for deciding if a CRTC will have FBC on it and then it will "lock" FBC on this CRTC: it won't be possible to change FBC's CRTC until disable(). With this, all checks and resource acquisition that only need to be done once per modeset can be moved from update() to enable(). And then the update(), activate() and deactivate() code will also get simpler since they won't need to worry about the CRTC being changed. The disable() function will do the reverse operation of enable(). One of its features is that it should only be called while the pipe is already off. This guarantees that FBC is stopped and nothing is using the CFB. With this, the activate() and deactivate() functions just start and temporarily stop FBC. They are the ones touching the hardware enable bit, so HW state reflects dev_priv->crtc.active. The last function remaining is update(). A lot of times I thought about renaming update() to activate() or try_to_activate() since it's called when we want to activate FBC. The thing is that update() may not only decide to activate FBC, but also deactivate or keep it on the same state, so I'll leave this name for now. Moving code to enable() and disable() will also help in case we decide to move FBC to pipe_config or something else later. The current patch only puts the very basic code on enable() and disable(). The next commits will take care of moving more stuff from update() to the new functions. v2: - Rebase. - Improve commit message (Chris). v3: Rebase after changing the patch order. v4: Rebase again after upstream changes. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
The long term goal is to have enable/disable as the higher level functions and activate/deactivate as the lower level functions, just like we do for PSR and for the CRTC. This way, we'll run enable and disable once per modeset, while update, activate and deactivate will be run many times. With this, we can move the checks and code that need to run only once per modeset to enable(), making the code simpler and possibly a little faster. This patch is just the first step on the conversion: it starts by converting the current low level functions from enable/disable to activate/deactivate. This patch by itself has no benefits other than making review and rebase easier. Please see the next patches for more details on the conversion. v2: - Rebase. - Improve commit message (Chris). v3: Rebase after changing the patch order. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
There's no need to reevaluate the status of every single crtc when a single crtc changes its state. With this, we're cutting the case where due to a change in pipe B, intel_fbc_update() is called, then intel_fbc_find_crtc() concludes FBC should be enabled on pipe A, then it completely rechecks the state of pipe A only to conclude FBC should remain enabled on pipe A. If any change on pipe A triggers a need to recompute whether FBC is valid on pipe A, then at some point someone is going to call intel_fbc_update(PIPE_A). The addition of intel_fbc_deactivate() is necessary so we keep track of the previously selected CRTC when we do invalidate/flush. We're also going to continue the enable/disable/activate/deactivate concept in the next patches. v2: Rebase. v3: Rebase after changing the patch order. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
This thing where we need to get the crtc either from the work structure or the fbc structure itself is confusing and unnecessary. Set fbc.crtc right when scheduling the enable work so we can always use it. The problem is not what gets passed and how to retrieve it. The problem is that when we're in the other parts of the code we always have to keep in mind that if FBC is already enabled we have to get the CRTC from place A, if FBC is scheduled we have to get the CRTC from place B, and if it's disabled there's no CRTC. Having a single place to retrieve the CRTC from allows us to treat the "is enabled" and "is scheduled" cases as the same case, reducing the mistake surface. I guess I should add this to the commit message. Besides the immediate advantages, this is also going to make one of the next commits much simpler. And even later, when we introduce enable/disable + activate/deactivate, this will be even simpler as we'll set the CRTC at enable time. So all the activate/deactivate/update code can just look at the single CRTC variable regardless of the current state. v2: Improve commit message (Chris). v3: Rebase after changing the patch order. Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
由 Paulo Zanoni 提交于
In function find_compression_threshold() we try to over-allocate CFB space in order to reduce reallocations and fragmentation, and we're not considering that at the CFB size check. Consider it. There is also a longer-term plan to kill dev_priv->fbc.uncompressed_size, but this will come later. v2: Use drm_mm_node_allocated() (Chris). Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
- 10 11月, 2015 4 次提交
-
-
由 Paulo Zanoni 提交于
From our maintainer Daniel Vetter a few days ago: "Oh dear this is dead code. kdbg uses the fbcon, which always uses untiled, which means fbc will never be enabled. Also we have 0 users and 0 test coverage for kdbg on top of i915 (Jesse implemented it for fun years back). Imo just remove all this code." Adding to what Daniel said: for kgdboc's KMS support, intel_pipe_set_base_atomic() already manually disables FBC, so we won't do the in_dbg_master() check there. This is essentially a revert of: commit c924b934 Author: Jason Wessel <jason.wessel@windriver.com> Date: Thu Aug 5 09:22:32 2010 -0500 i915: when kgdb is active display compression should be off Besides, it is not clear what is the exact problem caused by FBC, and why other features such as PSR, DRRS, IPS and RPM are not also checking for in_dbg_master(). IMHO we should either remove the code as suggested by Daniel or we add some nice comments explaining why is FBC so special. v2: Rebase due to new patch order. Cc: Jason Wessel <jason.wessel@windriver.com> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NJesse Barnes <jbarnes@virtuousgeek.org> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-13-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Daniel was looking at this code and asked about whether fb->pitches[0] is correct, then he suggested we should a comment to make sure it is actually intentional. For more information on the CFB size calculation, please see the commit message of: commit c4ffd409 Author: Paulo Zanoni <paulo.r.zanoni@intel.com> Date: Thu Oct 1 19:55:57 2015 -0300 drm/i915: fix CFB size calculation Requested-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-12-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
If we run igt/kms_frontbuffer_tracking, this message will appear thousands of times, eating a significant part of our dmesg buffer. It's part of the expected FBC behavior, so let's just silence it. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-10-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Make sure we deactivate FBC at intel_fbc_init(), so we can remove the call from intel_display.c. Currently we only have the "enabled" software state, but later we'll have both "enabled" and "active", and we'll add assertions to them, so just calling intel_fbc_disable() from intel_modeset_init() won't work. It's better to make sure intel_fbc_init() already puts the hardware in the expected state, so we can put nice assertions in the other functions. v2: Keep/improve the comment (Chris). v3: Improve the commit message a little bit. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-9-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-