- 03 Dec, 2015 6 commits
-
-
Paulo Zanoni authored
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:Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/
-
Paulo Zanoni authored
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:
-
Paulo Zanoni authored
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:
-
Paulo Zanoni authored
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:
-
Paulo Zanoni authored
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:
-
Paulo Zanoni authored
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:
-
- 02 Dec, 2015 15 commits
-
-
Jani Nikula authored
Choose between i2c bit banging and gmbus in a new higher level function, and let the i2c core retry the first time we fall back to bit banging. The i2c core imposes a timeout on -EAGAIN, but it defaults to 1 second, and shouldn't be a problem for us. Reviewed-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1448980166-23055-2-git-send-email-jani.nikula@intel.com
-
Jani Nikula authored
Shorter, easier to follow code with no functional changes. In all cases, the return value ultimately comes from gmbus_wait_hw_status() anyway. Reviewed-by:
-
Ville Syrjälä authored
While not technically needed on the last case in the switch statement, the 'break' makes it look better IMO. v2: Fixed a typo in the commit message (Paulo) Signed-off-by:
-
Ville Syrjälä authored
Unfortunatey there appear to quite a few HSW/BDW machines (eg. NUCs, Brix Pro) in the wild with LPT/WPT-H that have no physical CRT connector and non-working FDI. FDI training fails every single time on these machines. Dunno, maybe they just didn't bother wiring it up or something? Unfortunately all the fuse bits and whatnot are telling us that the CRT connector is present. And so what we get from this is tons of false positives from the CI systems due to VGA connector forcing. I've not found any way to detect this purely from hardware, so we have to resort to looking at the VBT int_crt_support bit. We used to check this bit on all platforms, but that broke all the old machines, so the check was then restricted to VLV only in commit 84b4e042 ("drm/i915: only apply crt_present check on VLV") Considering HSW and VLV VBT probably got defined around the same time, it should be reasonably safe to assume that the bits is sane for HSW/BDW as well. At least I have one copy of some VBT spec here that says it's meant for both VLV and HSW, and it knows about the bit (lists it being valid from version 155 onwards). Also I have two desktop machines with actual CRT ports and both have int_crt_support==1 in their VBTs. Also we already trust VBT >= 155 to tell us various details about the DDI ports, so trusting it a bit more seems reasonable. As far as VLV goes, the added VBT version check should be fine. Even if someone has some weird VLV machine with a very old VBT version, it just means they'll end up with a shadow CRT connector. IIRC the reason for eliminating the shadow CRT connector on VLV was to speed up display probing rather than fixing something more serious. v2: Move the platform checks into the VBT parsing code Also check that the VBT version is at least 155 v3: Improve commit message (Paulo) Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by:
-
Ville Syrjälä authored
On HSW/BDW DDI A and E share 2 lanes, so when DDI A requires the shared lanes DDI E can't be used. The lanes are not supposed to be dynamically switched between the two uses, so there's no point in registering the CRT connector when DDI E has no lanes. v2: Fix typos in the commit message (Paulo) Signed-off-by:
-
Ville Syrjälä authored
LPT-H has a strap bit for fused off CRT block. Check it to see if we should register the CRT connector or not. Supposedly this also forces the ADAP enable bit to 0, so the detection we added in commit 6c03a6bd ("drm/i915: Don't register CRT connector when it's fused off") should already catch it, but checking the fuse bit should at least do no harm. v2: Use HAS_PCH_LPT_H() (Paulo) Signed-off-by:
-
Ville Syrjälä authored
No need to read out cdclk from the hardware, we have it already cached in dev_priv. Signed-off-by:
-
Ville Syrjälä authored
Currently we round the AUX clock divider down on g4x, to closest on HSW/BDW port A, and up everywhere else. We are supposed to get as close to 2MHz as we can, so round to closest seems like the best option. Signed-off-by:
-
Ville Syrjälä authored
Somehow we accumulated a duplicated .get_display_clock_speed() assignment for PNV in commit 34edce2f ("drm/i915: Add cdclk extraction for g33, g965gm and g4x") No real harm on having two, we just never reach the second one, so simply kill it. Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by:
-
Ville Syrjälä authored
We have HAS_PCH_LPT_LP() already, so add HAS_PCH_LPT_H() and use it where appropriate. Signed-off-by:
-
Ville Syrjälä authored
CTG uses hrawclk for backlight, so calculate the max based on that instead of cdclk. Fixes: aa17cdb4 ("drm/i915: initialize backlight max from VBT") Signed-off-by:
-
Ville Syrjälä authored
Convert the MHz number coming from intel_rawclk() into Hz in i9xx_hz_to_pwm() on PNV. Otherwise we'll get something totally bogus as a result. Fixes: aa17cdb4 ("drm/i915: initialize backlight max from VBT") Signed-off-by:
-
Imre Deak authored
Per bspec, "Backlight PWM may stop in the asserted state, causing backlight to stay fully on. WA: Before disabling PWM, set CLKGATE_DIS_0 0x46530 bit 13 PWM1 Gating Dis (for PWM1) or bit 14 PWM2 Gating Dis (for PWM2). The bits can remain set without harm." (There's no workaround name for this.) This fixes some Broxton backlight issues. Signed-off-by:
Imre Deak <imre.deak@intel.com> Reviewed-by:
-
Jani Nikula authored
If the backlight modulation frequency can't be extracted from the registers or from VBT, use 200 Hz as the default. This may enable backlight on some machines that previously failed. Reviewed-by:
-
Jani Nikula authored
The only missing piece is the function to convert frequency to PWM register value. The PWM is based on 19.2 MHz clock, except for BXT A step, which is based on CDCLK, and which we ignore. Reviewed-by:
-
- 01 Dec, 2015 4 commits
-
-
Matt Roper authored
BXT power well support is not yet stable. Starting with patch commit 9f836f90 Author: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> Date: Mon Nov 16 16:20:01 2015 +0100 drm/i915/gen9: Turn DC handling into a power well DPMS off operations may actually cause the entire system to powerdown or reboot. Disable power well support for now until Broxton gets fixes similar to what we have for SKL. Cc: Imre Deak <imre.deak@intel.com> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> References: http://lists.freedesktop.org/archives/intel-gfx/2015-November/081037.htmlSigned-off-by:Matt Roper <matthew.d.roper@intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1448990818-11005-1-git-send-email-matthew.d.roper@intel.comReviewed-by:
-
Ville Syrjälä authored
ironlake_crtc_compute_clock() gets called during atomic compute phase, so we must check the future pipe type instead of the current type. Signed-off-by:
-
Ville Syrjälä authored
The .get_config() hooks should not reference anything in crtc->config, everything should be based on the passed in pipe_config instead. So don't dig out the cpu_transcoder from crtc->config on ddi platfforms, and also avoid using the encoder->crtc link and instead look up the pipe via pipe_config->base.crtc. I don't think this will actually fix anything since during the initial state readout we set up the encoder->crtc link prior to calling .get_config(), and during the modeset state check the encoder->crtc ought to be correct anyway since it's that state we just programmed. But this seems the right thing to do anyway. While at it, do some house cleaning on the local variables in the .infoframe_enabled() hooks. Signed-off-by:
-
Daniel Vetter authored
Similar to commit 37ca8d4c Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Fri Oct 30 19:20:27 2015 +0200 drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB we can only enable fifo underrun reporting when using the fdi/lpt after everything is set up and after a bit of waiting. The waiting is required, enabling it right after enabling encoders will first trigger an underrun on the pch and then, 1 frame later, an underrun on the cpu. Two vblank waits after encoder enabling seems enough to curb it. And similar to Author: Ville Syrjälä <ville.syrjala@linux.intel.com> Date: Fri Nov 20 22:09:18 2015 +0200 drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB we also need to make sure cpu fifo underrun reporting is disabled when enabling the fdi rx/tx and pch transcoder&port. But somehow this is only needed when enabling, not also when disabling. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1448705139-12534-1-git-send-email-daniel.vetter@ffwll.ch Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91578Tested-by:
Mika Kahola <mika.kahola@intel.com> Reviewed-by:
-
- 30 Nov, 2015 4 commits
-
-
Jani Nikula authored
DSI has quite a few special cases, like DP, so add it to crtc state. This way we can get rid of a number of intel_pipe_has_type() checks for DSI. This isn't necessarily the prettiest way, but it's a step towards being aligned with what's being done with other encoders. Reviewed-by:
-
Jani Nikula authored
The hook was added to cater for DSI, but with the hooks rearranged on the DSI encoder side, this is no longer needed. It was a bit silly anyway to have two hooks called back-to-back. Reviewed-by:
-
Jani Nikula authored
For DSI, the pre_pll_enable and the pre_enable hooks are called back-to-back on all platforms that support DSI. The distinction is artificial for DSI, for which we enable the DSI PLL in the encoder hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook. Reviewed-by:
-
Namrta Salonie authored
Found by static code analysis tool. v2: Inserted block instead of goto & renamed variables (Chris) v3: Aligned code as per the opening brace (Chris) Rebased on top of nightly (Daniel) Signed-off-by:Namrta Salonie <namrta.salonie@intel.com> Signed-off-by:
Deepak S <deepak.s@intel.com> Signed-off-by:
-
- 26 Nov, 2015 7 commits
-
-
Ville Syrjälä authored
Signed-off-by:
-
Ville Syrjälä authored
Pull the BDW+ DE pipe interrupt mask frobbing into a central place, like we have for other platforms. v2: Fix the kerneldoc (Daniel) Signed-off-by:
-
Ville Syrjälä authored
ironlake_{enable,disable}_display_irq() each just call ilk_update_display_irq() so let's make them static inlines. While at it s/ironlake/ilk/ to make things shorter, and a bit more consistent with the ibx functions. Signed-off-by: -
Ville Syrjälä authored
No reason why ibx_{enable,disable}_display_interrupt() couldn't be static inlines instead of cpp macros. Signed-off-by: -
Jani Nikula authored
This reverts commit 97e5ed11 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Fri Oct 23 10:56:12 2015 +0200 drm/i915: shut up gen8+ SDE irq dmesg noise With the proper fix ("drm/i915: fix the SDE irq dmesg warnings properly") reliably in place, bring back the error message. Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by:
-
Jani Nikula authored
We had the "The master control interrupt lied (SDE)!" check and error message in place for a long time without any problems, until commit aaf5ec2e Author: Sonika Jindal <sonika.jindal@intel.com> Date: Wed Jul 8 17:07:47 2015 +0530 drm/i915: Handle HPD when it has actually occurred caused the errors to start happening. This was bisected and reported, but the error message was silenced in commit 97e5ed11 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Fri Oct 23 10:56:12 2015 +0200 drm/i915: shut up gen8+ SDE irq dmesg noise shooting the messenger while the debugging for why Sonika's commit triggered the errors was still in progress. It looks like we need to read and acknowledge the PCH_PORT_HOTPLUG register even though the hotplug trigger indicates there isn't a hotplug irq to handle. The PCH doesn't seem to really ack the the interrupt to the CPU unless we touch the hotplug register. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Sonika Jindal <sonika.jindal@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Reviewed-by:
-
Gerd Hoffmann authored
Commit "30c964a6 drm/i915: Detect virtual south bridge" detects and handles the southbridge emulated by vmware esx. Add the ich9 south bridge emulated by 'qemu -M q35'. Signed-off-by:
Gerd Hoffmann <kraxel@redhat.com> Signed-off-by:
-
- 24 Nov, 2015 4 commits
-
-
Jani Nikula authored
commit b04c5bd6 Author: Borun Fu <borun.fu@intel.com> Date: Sat Jul 12 10:02:27 2014 +0530 drm/i915: Power gating display wells during i915_pm_suspend moved for_each_power_domain from intel_display.c to i915_drv.h but we still have the definition around in intel_display.c, due to a merge conflict resolution gone wrong in commit 4dac3edf Merge: 48777767 e05444be Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Tue Jul 29 20:49:36 2014 +0200 Merge remote-tracking branch 'airlied/drm-next' into drm-intel-next Just remove the extra definition left behind. Reviewed-by:
-
Ville Syrjälä authored
On some machines the CRT connector may be fused off. The weird thing about this setup is that the ADPA register works otherwise normally, except the enable bit is hardwired to 0. No one knows of any fuse register that would tell us if this is the case, so the only thing we can do (apart from a blacklist) is to try and set the enable bit and see if it sticks. If not, we don't register the connector at all. Obviously if the bit is already set when loading the driver we can just assume it works. I've smoke tested this approach on several machines (GMCH and PCH), some with actual CRT connectors, some with shadow connectors, and obviously the machine (IVB) with the fused off connector. So far I've not seen any ill effects from this probe. The main benefit is that we can actually run igt on machines with fused off connectors, without totally upsetting the state checker. Signed-off-by:
-
Ville Syrjälä authored
To get a better idea if underruns occurred during crtc disabling, let's check for them explicitly. This helps in cases where the error interrupt isn't active, or there is no underrun interrupt support at all. Signed-off-by:
-
Ville Syrjälä authored
We sometimes get a spurious CPU pipe underrun somewhere between enabling port A and enabling vdd for the panel. Observed on both ILK and IVB with port A eDP. Suppress FIFO underrun reporting around the port and vdd enable to avoid the dmesg errors. Not sure if port D eDP would suffer from the same issue, but assume that it doesn't until proven differently. Testcase: igt/kms_setmode Signed-off-by:
-
