- 13 May, 2016 40 commits
-
-
Ville Syrjälä authored
All the fields in CDCLK_CTL we don't program should be left at zero, so let's just get rid of the RMW. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1462995892-32416-14-git-send-email-ville.syrjala@linux.intel.comReviewed-by:
Imre Deak <imre.deak@intel.com>
-
Ville Syrjälä authored
BXT could change the CD2X divider synchronized with a single pipe. So assuming the DE PLL frequency doesn't need to be changed, we could change cdclk without shutting off the pipe (when only a single pipe is enabled). In the meantime let's configure CDCLK_CTL for non-double buffered CD2X update, although it shouldn't really matter as long as the selected pipe is disabled when reprogramming the divider. Signed-off-by:
-
Ville Syrjälä authored
The 'required' part of 'required_vco' should be obvious. Let's just call it 'vco' for brevity. Signed-off-by:
Jani Nikula <jani.nikula@intel.com>
-
Ville Syrjälä authored
Rename the generic sounding freq/frequency parameters to the cdclk functions to 'cdclk' so that we'll know which clock we're talking about once we have to deal with the vco frequencies as well. Signed-off-by:
-
Ville Syrjälä authored
We calculate the CDCLK_CTL value from scratch so no need to attempt some form of RMW first. Signed-off-by:
-
Ville Syrjälä authored
Make thins a bit easier to read by extracting the SKL DPLL0 disable into separate functions. We already have the enable counterpart. Down the line this will also help make the cdclk programming on SKL, BXT, and following platforms look rather consistent. Signed-off-by:
-
Ville Syrjälä authored
We don't need any pixel clock vs. cdclk guardband since HSW. BXT still tries to add one though. Get rid of it. Signed-off-by:
-
Ville Syrjälä authored
Both SKL and BXT need to fill in the "decimal" cdclk frequency into the CDCLK_CTL register. SKL uses a small helper to do the kHz->"decimal" conversion, whereas BXT has it open-coded. Use the helper on BXT too. While at it, change it to round to closest rather than down. It doesn't actually matter with the frequencies we have to deal with, but it seems like the right thing to do. Signed-off-by:
-
Ville Syrjälä authored
BXT uses the "pch" panel fitter configuration, so we can use ilk_max_pixel_rate() instead of intel_mode_max_pixclk() to compute the pipe pixel rate. ilk_max_pixel_rate() will account for the pipe scaler downscaling factor whereas intel_mode_max_pixclk() will not. I'm pretty sure the same limitation is there on GMCH platforms, but no one just bothered to implement the downscaling adjustment for them. Probably should just unify the panel fitter setup more across the platforms and use the exact same code on all platforms for this. But in the meantime, let's at least make BXT a bit more correct. Signed-off-by:
-
Ville Syrjälä authored
broxton_calc_cdclk() doesn't need dev_priv for anything, so let's not bother passing it around. Signed-off-by:
-
Ville Syrjälä authored
Split the .fdi_link_train and .modeset_commit_cdclk/.modeset_calc_cdclk into two separate if ladders. Much easier to read when you're not confusing two totally separate subjects. Signed-off-by:
-
Ville Syrjälä authored
Try to reduce the amount of duplicated cdclk magic numbers by moving the max_pixclk->cdclk conversion into a helper. Signed-off-by:
-
Ville Syrjälä authored
commit 565602d7 ("drm/i915: Do not acquire crtc state to check clock during modeset, v4.") removed the possibility that intel_mode_max_pixclk() or ilk_max_pixel_rate() might return an error, so let's get rid of the error checks in the callers as well. Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Mika Kahola <mika.kahola@intel.com> Signed-off-by:
-
Matt Roper authored
We calculate the watermark config into intel_atomic_state and then save it into dev_priv, but never actually use it from there. This is left-over from some early ILK-style watermark programming designs that got changed over time. Signed-off-by:
Matt Roper <matthew.d.roper@intel.com> Reviewed-by:
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1463061971-19638-18-git-send-email-matthew.d.roper@intel.com
-
Matt Roper authored
If we can't find any valid level 0 watermark values for the requested atomic transaction, reject the configuration before we try to start programming the hardware. v2: - Add extra debugging output when we reject level 0 watermarks so that we can more easily debug how/why they were rejected. Cc: Lyude Paul <cpaul@redhat.com> Signed-off-by:
-
Matt Roper authored
Moving watermark calculation into the check phase will allow us to to reject display configurations for which there are no valid watermark values before we start trying to program the hardware (although those tests will come in a subsequent patch). Another advantage of moving this calculation to the check phase is that we can calculate the watermarks in a single shot as part of the atomic transaction. The watermark interfaces we inherited from our legacy modesetting days are a bit broken in the atomic design because they use per-crtc entry points but actually re-calculate and re-program something that is really more of a global state. That worked okay in the legacy modesetting world because operations only ever updated a single CRTC at a time. However in the atomic world, a transaction can involve multiple CRTC's, which means we wind up computing and programming the watermarks NxN times (where N is the number of CRTC's involved). With this patch we eliminate the redundant re-calculation of watermark data for atomic states (which was the cause of the WARN_ON(!wm_changed) problems that have plagued us for a while). We still need to work on the 'commit' side of watermark handling so that we aren't doing redundant NxN programming of watermarks, but that's content for future patches. v2: - Bail out of skl_write_wm_values() if the CRTC isn't active. Now that we set dirty_pipes to ~0 if the active pipes change (because we need to deal with DDB changes), we can now wind up here for disabled pipes, whereas we couldn't before. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89055 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181 Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
Daniel Stone <daniels@collabora.com> Reviewed-by:
-
Matt Roper authored
Once we move watermark calculation to the atomic check phase, we'll want to start rejecting display configurations that exceed out watermark limits. At the moment we just assume that there's always a valid set of watermarks, even though this may not actually be true. Let's prepare by passing return codes up through the call stack in preparation. Signed-off-by:
-
Matt Roper authored
Slightly easier to work with than an array of bools. Signed-off-by:
-
Matt Roper authored
In an upcoming patch we'll move this calculation to the atomic 'check' phase so that the display update can be rejected early if no valid watermark programming is possible. v2: - Drop intel_pstate_for_cstate_plane() helper and add note about how the code needs to evolve in the future if we start allowing more than one pending commit against a CRTC. (Maarten) v3: - Only have skl_compute_wm_level calculate watermarks for enabled planes; we can just set the other planes on a CRTC to disabled without having to look at the plane state. This is important because despite our CRTC lock we can still have racing commits that modify a disabled plane's property without turning it on. (Maarten) Signed-off-by:
-
Matt Roper authored
In a future patch we'll want to calculate plane watermarks for in-flight atomic state rather than the already-committed state. Signed-off-by:
-
Matt Roper authored
Now that we're properly pre-allocating the DDB during the atomic check phase and we trust that the allocation is appropriate, let's actually use the allocation computed and not duplicate that work during the commit phase. v2: - Significant rebasing now that we can use cached data rates and minimum block allocations to avoid grabbing additional plane states. Signed-off-by:
-
Matt Roper authored
Calculate the DDB blocks needed to satisfy the current atomic transaction at atomic check time. This is a prerequisite to calculating SKL watermarks during the 'check' phase and rejecting any configurations that we can't find valid watermarks for. Due to the nature of DDB allocation, it's possible for the addition of a new CRTC to make the watermark configuration already in use on another, unchanged CRTC become invalid. A change in which CRTC's are active triggers a recompute of the entire DDB, which unfortunately means we need to disallow any other atomic commits from racing with such an update. If the active CRTC's change, we need to grab the lock on all CRTC's and run all CRTC's through their 'check' handler to recompute and re-check their per-CRTC DDB allocations. Note that with this patch we only compute the DDB allocation but we don't actually use the computed values during watermark programming yet. For ease of review/testing/bisecting, we still recompute the DDB at watermark programming time and just WARN() if it doesn't match the precomputed values. A future patch will switch over to using the precomputed values once we're sure they're being properly computed. Another clarifying note: DDB allocation itself shouldn't ever fail with the algorithm we use today (i.e., we have enough DDB blocks on BXT to support the minimum needs of the worst-case scenario of every pipe/plane enabled at full size). However the watermarks calculations based on the DDB may fail and we'll be moving those to the atomic check as well in future patches. v2: - Skip DDB calculations in the rare case where our transaction doesn't actually touch any CRTC's at all. Assuming at least one CRTC state is present in our transaction, then it means we can't race with any transactions that would update dev_priv->active_crtcs (which requires _all_ CRTC locks). v3: - Also calculate DDB during initial hw readout, to prevent using incorrect bios values. (Maarten) v4: - Use new distrust_bios_wm flag instead of skip_initial_wm (which was never actually set). - Set intel_state->active_pipe_changes instead of just realloc_pipes Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Lyude Paul <cpaul@redhat.com> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com> Signed-off-by:
-
Matt Roper authored
SKL-style platforms can't fully trust the watermark/DDB settings programmed by the BIOS and need to do extra sanitization on their first atomic update. Add a flag to dev_priv that is set during hardware readout and cleared at the end of the first commit. Note that for the somewhat common case where everything is turned off when the driver starts up, we don't need to bother with a recompute...we know exactly what the DDB should be (all zero's) so just setup the DDB directly in that case. v2: - Move clearing of distrust_bios_wm up below the swap_state call since it's a more natural / self-explanatory location. (Maarten) - Use dev_priv->active_crtcs to test whether any CRTC's are turned on during HW WM readout rather than trying to count the active CRTC's again ourselves. (Maarten) Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by:
-
Matt Roper authored
We eventually want to calculate watermark values at atomic 'check' time instead of atomic 'commit' time so that any requested configurations that result in impossible watermark requirements are properly rejected. The first step along this path is to allocate the DDB at atomic 'check' time. As we perform this transition, allow the main allocation function to operate successfully on either an in-flight state or an already-commited state. Once we complete the transition in a future patch, we'll come back and remove the unnecessary logic for the already-committed case. v2: Rebase/refactor; we should no longer need to grab extra plane states while allocating the DDB since we can pull cached data rates and minimum block counts from the CRTC state for any planes that aren't being modified by this transaction. v3: - Simplify memsets to clear DDB plane entries. (Maarten) - Drop a redundant memset of plane[pipe][PLANE_CURSOR] that was added by an earlier Coccinelle patch. (Maarten) - Assign *num_active at the top of skl_ddb_get_pipe_allocation_limits() so that no code paths return without setting it. (kbuild robot) Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: -
Matt Roper authored
For the purposes of DDB re-allocation we need to know whether a transaction changes the list of CRTC's that are active. While state->modeset could be used for this purpose, that would be slightly too aggressive since it would lead us to re-allocate the DDB when a CRTC's mode changes, but not its final active state. Signed-off-by:
-
Matt Roper authored
This will eventually allow us to re-use old values without re-calculating them for unchanged planes (which also helps us avoid re-grabbing extra plane states). v2: - Drop unnecessary memset's; they were meant for a later patch (which got reworked anyway to not need them, but were mis-rebased into this one. (Maarten) Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: -
Matt Roper authored
Our skl_get_total_relative_data_rate() function gets passed a crtc state object to calculate the data rate for, but it currently always looks up the committed plane states that correspond to that CRTC. Let's check whether the CRTC state is an in-flight state (meaning cstate->state is non-NULL) and if so, use the corresponding in-flight plane states. We'll soon be using this function exclusively for in-flight states; at that time we'll be able to simplify the function a bit, but for now we allow it to be used in either mode. v2: - Rebase on top of changes to cache plane data rates. Signed-off-by:
-
Matt Roper authored
This will be important when we start calculating CRTC data rates for in-flight CRTC states since it will allow us to calculate the total data rate without needing to grab the plane state for any planes that aren't updated by the transaction. Signed-off-by:
-
Matt Roper authored
When we added atomic watermarks, we added a new display vfunc 'compute_pipe_wm' that is used to compute any pipe-specific watermark information that we can at atomic check time. This was a somewhat poor naming choice since we already had a 'skl_compute_pipe_wm' function that doesn't quite fit this model --- the existing SKL function is something that gets used at atomic commit time, after the DDB allocation has been determined. Let's rename the existing SKL function to avoid confusion. Signed-off-by:
-
Matt Roper authored
Reorganize the nested structures and unions we have for pipe watermark data in intel_crtc_state so that platform-specific data can be added in a more sensible manner (and save a bit of memory at the same time). The change basically changes the organization from: union { struct intel_pipe_wm ilk; struct intel_pipe_wm skl; } optimal; struct intel_pipe_wm intermediate /* ILK-only */ to union { struct { struct intel_pipe_wm intermediate; struct intel_pipe_wm optimal; } ilk; struct { struct intel_pipe_wm optimal; } skl; } There should be no functional change here, but it will allow us to add more platform-specific fields going forward (and more easily extend to other platform types like VLV). While we're at it, let's move the entire watermark substructure out to its own structure definition to make the code slightly more readable. Signed-off-by: -
Imre Deak authored
Marking function return types as const is redundant, as these are rvalues and as such constant by definition. Code checkers and GCC will warn about this so remove the modifier. Signed-off-by:
-
Imre Deak authored
A failure from these functions can lead to obscure bugs later, so it's better not to suppress them and just fail module loading. Signed-off-by:
-
Imre Deak authored
Code checkers may complain about the explicit casts between different enum types, so add comments for known-valid cases to help future triaging of such complaints. v2: - Make the comments more logical (Ville). Signed-off-by:
-
Imre Deak authored
scaler_id may be negative as shown by conditions later in the function, so don't use it as an array index in that case. v2: - Remove ps_ctrl while at it (Ville). Signed-off-by:
-
Chris Wilson authored
Following a GPU hang, we break out of the request loop in order to unlock the struct_mutex for use by the GPU reset. However, if we retire all the requests at that moment, we cannot identify the guilty request after performing the reset. v2: Not automatically retiring requests forces us to recheck for available ringspace. Fixes: f4457ae7 ("drm/i915: Prevent leaking of -EIO from i915_wait_request()") Testcase: igt/gem_reset_stats/ban-* Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Mika Kuoppala <mika.kuoppala@intel.com> Tested-by:
Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by:
-
Chris Wilson authored
In order to reduce the workload of the caller, we do not want to actually have to retire requests of others when checking the busy status of this object. This applies to both busy/wait ioctls as the wait ioctl has a semantically equivalent mode to the busy ioctl. At the present time, this is only a minor improvement to reduce the workload of the busy ioctl under the struct_mutex which helps to reduce its impact upon contention of struct_mutex. However, since it is mostly a victim in highly contended scenarios, the impact is very minor until we can eliminate the struct_mutex requirement for busy-ioctl in the near future. v2: Mention the patches intended limited impact. It is just paving the way for greater changes whilst reducing the impact of a bugfix in the next patch. Signed-off-by:
-
Chris Wilson authored
The get-reset-stats ioctls wasn't waiting for a pending reset before reporting its statistics, and so was ignoring a hang generated by the context that should have been reported against said context. Signed-off-by:
-
Chris Wilson authored
The get-reset-stats ioctl reports upon the statistics (number of hangs, be it as a victim or the guilty party) of a particular context. It is semantically better as being part of i915_gem_context.c user interface, as opposed to the hardware level access of intel_uncore.c Signed-off-by:
-
Ander Conselvan de Oliveira authored
The coding style documentation says the following about typedefs: "In general, a pointer, or a struct that has elements that can reasonably be directly accessed should _never_ be a typedef." intel_limit_t falls in that category, so just use "struct intel_limit" instead. Signed-off-by:
Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Reviewed-by:
-
Ander Conselvan de Oliveira authored
Those are only used for defining struct intel_limit, so use anonymous structs instead. Signed-off-by:
-
