- 30 1月, 2016 4 次提交
-
-
由 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 12 次提交
-
-
由 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>
-
由 Paulo Zanoni 提交于
If FBC is disabled we will still call intel_fbc_invalidate(), and as a result we may call intel_fbc_deactivate(), which will try to touch registers. I'm pretty sure I saw this happen on a runtime suspended device, and I'm almost sure I was running igt/pm_rpm. It produced the "you touched registers while the device is suspended" WARNs. But this was some time ago and I can't remember exactly which conditions were necessary to reproduce the problem. v2: Rebase to new series order. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-8-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Don't try to list in comments the cases where we should enable or disable FBC: it varies a lot with the hardware generations and the code should be the documentation. Also notice that there's already a huge gap between the comments and what's in the code. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-7-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
This change was part of the commit that makes intel_fbc_update() receive an intel_crtc as argument instead of dev_priv, but since it was polluting the diff with too many chunks I decided to move it to its own commit. It seems that our developers are favoring having this instead of the old combination drm_crtc *crtc + intel_crtc *intel_crtc, and on the mentioned commit we'll get rid of the drm_crtc variable, so let's do an intermediate commit with the rename, so on the next commit we'll have just struct intel_crtc *crtc. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-6-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
We're going to kill intel_fbc_find_crtc(), that's why a big part of the logic moved from intel_fbc_find_crtc() to crtc_is_valid(). v2: - Rebase due to pipe_a_only change. - Split the multiline conditional (Chris). Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-5-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
We already check if the CRTC is visible, and it shouldn't be possible to have a visible CRTC without an FB. This was noticed by both Chris and Ville on different ocasions. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-4-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Make the code easier to read. Suggested-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/1446664257-32012-3-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Although the term "nuke" is part of the FBC spec, it's not very intuitive, so let's rename it to make it easier for people that are not familiar with the spec. Requested-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/1446664257-32012-2-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
由 Paulo Zanoni 提交于
Newlines are not needed and they're not used by the other messages. I added the newline by mistake. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1446664257-32012-14-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
- 04 11月, 2015 1 次提交
-
-
由 Paulo Zanoni 提交于
I wanted to add yet another check to intel_fbc_update() and realized I would need to create yet another enum no_fbc_reason case. So I remembered this patch series that Damien wrote a long time ago and nobody ever reviewed, so I decided to reimplement it since the code changed a lot since then. Credits-to: Damien Lespiau <damien.lespiau@intel.com> Cc: Damien Lespiau <damien.lespiau@intel.com> Reviewed-by: NDaniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1445964628-30226-2-git-send-email-paulo.r.zanoni@intel.comSigned-off-by: NMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
-
- 29 10月, 2015 1 次提交
-
-
由 Rodrigo Vivi 提交于
Kabylake is a Intel® Processor containing Intel® HD Graphics following Skylake. It is Gen9p5, so it inherits everything from Skylake. Let's start by adding the platform separated from Skylake but reusing most of all features, functions etc. Later we rebase the PCI-ID patch without is_skylake=1 so we don't replace what original Author did there. Few IS_SKYLAKEs if statements are not being covered by this patch on purpose: - Workarounds: Kabylake is derivated from Skylake H0 so no W/As apply here. - GuC: A following patch removes Kabylake support with an explanation: No firmware available yet. - DMC/CSR: Done in a separated patch since we need to be carefull and load the version for revision 7 since Kabylake is Skylake H0. v2: relative cleaner commit message and added the missed IS_KABYLAKE to intel_i2c.c as pointed out by Jani. Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: NRodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: NJani Nikula <jani.nikula@intel.com>
-
- 09 10月, 2015 2 次提交
-
-
由 Paulo Zanoni 提交于
According to my experiments (and later confirmation from the hardware developers), the maximum sizes mentioned in the specification delimit how far in the buffer the hardware tracking can go. And the hardware calculates the size based on the plane address we provide - and the provided plane address might not be the real x:0,y:0 point due to the compute_page_offset() function. On platforms that do the x/y offset adjustment trick it will be really hard to reproduce a bug, but on the current SKL we can reproduce the bug with igt/kms_frontbuffer_tracking/fbc-farfromfence. With this patch, we'll go from "CRC assertion failure" to "FBC unexpectedly disabled", which is still a failure on the test suite but is not a perceived user bug - you will just not save as much power as you could if FBC is disabled. v2, rewrite patch after clarification from the Hadware guys: - Rename function so it's clear what the check is for. - Use the new intel_fbc_get_plane_source_sizes() function in order to get the proper sizes as seen by FBC. v3: - Rebase after the s/sizes/size/ on the previous patch. - Adjust comment wording (Ville). - s/used_/effective_/ (Ville). Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL) Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
We were considering the whole framebuffer height, but the spec says we should only consider the active display height size. There were still some unclear questions based on the spec, but the hardware guys clarified them for us. According to them: - CFB size = CFB stride * Number of lines FBC writes to CFB - CFB stride = plane stride / compression limit - Number of lines FBC writes to CFB = MIN(plane source height, maximum number of lines FBC writes to CFB) - Plane source height = - pipe source height (PIPE_SRCSZ register) (before SKL) - plane size register height (PLANE_SIZE register) (SKL+) - Maximum number of lines FBC writes to CFB = - plane source height (before HSW) - 2048 (HSW+) For the plane source height, I could just have made our code do I915_READ() in order to be more future proof, but since it's not cool to do register reads I decided to just recalculate the values we use when we actually write to those registers. With this patch, depending on your machine configuration, a lot of the kms_frontbuffer_tracking subtests that used to result in a SKIP due to not enough stolen memory still start resulting in a PASS. v2: Use the clipped src size instead of pipe_src_h (Ville). v3: Use the appropriate information provided by the hardware guys. v4: Bikesheds: s/sizes/size/, s/fb_cpp/cpp/ (Ville). v5: - Don't use crtc->config->pipe_src_x for BDW- (Ville). - Fix the register name written in the comment. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 30 9月, 2015 2 次提交
-
-
由 Paulo Zanoni 提交于
Make the giant function a little less giant. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> [danvet: Add pipe_ prefix as suggested by Chris.] Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
Make it clear that we're checking whether FBC is supported or not. The fact that the vfunc is not NULL is just a consequence. Another name option would have been fbc_initialized(). Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NChris Wilson <chris@chris-wilson.co.uk> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
- 23 9月, 2015 7 次提交
-
-
由 Ville Syrjälä 提交于
Signed-off-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: NJani Nikula <jani.nikula@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
I only tested this on BDW and SKL, but since the register description is the same ever since gen4, let's assume that all gens take the same register format. If that's not true, then hopefully someone will bisect a bug to this patch and we'll fix it. Notice that the wrong fence offset register just means that the hardware tracking will be wrong. Testcases: - igt/kms_frontbuffer_tracking/fbc-1p-primscrn-pri-shrfb-draw-mmap-gtt - igt/kms_frontbuffer_tracking/fbc-2p-primscrn-pri-shrfb-draw-mmap-gtt v2: - Add intel_crtc->adjusted_{x,y} so this code can work independently of intel_gen4_compute_page_offset(). (Ville). - This version also works on SKL. Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
This commit is essentially a rewrite of "drm/i915: Check pixel format for fbc" from Ville Syrjälä. The idea is the same, but the code is different due to all the changes that happened since his original patch. So any bugs are due to my bad rewrite. v2: - Drop the alpha formats (Ville). v3: - Drop the stale comment (Ville). Testcases: igt/kms_frontbuffer_tracking/*fbc*-${format_name}-draw-* Credits-to: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
This WA is only for HSW/BDW. Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
The spec says the register should have that value for the entire time that FBC is enabled, so apply the WA before we enable FBC. Notice that we also have this WA for ILK/SNB, but it is implemented at init_clock_gating(). I could move the IVB/HSW/BDW WA code to init_clock_gating() too, but since we recently had some complaints about WAs not staying after being set, I'm going to play safe and keep this here for now. Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
BSpec says we shouldn't enable FBC on HSW/BDW when the pipe pixel rate exceeds 95% of the core display clock. v2: - HSW also needs the WA (Ville). - Add the WA name (Ville). - Use the current cdclk (Ville). Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-
由 Paulo Zanoni 提交于
And also print the threshold. I was surprised to see a log message claiming the CFB size was 32mb when there was less than 24mb available for it. Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: NPaulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: NDaniel Vetter <daniel.vetter@ffwll.ch>
-