Re: [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mc...@intel.com wrote:
> From: Jeff McGee
>
> New test core_getparams consists of 2 subtests, each one testing
> the ability of userspace to query the correct value of a GT config
> attribute: subslice total or EU total. drm/i915 implementation of
> these queries is required for Cherryview and Gen9+ devices (non-
> simulated).
>
> v2: Duplicate small amount of new libdrm functionality to avoid
> bumping libdrm version requirement (Daniel). Convert some
> igt_asserts to the appropriate comparison variants. Add a
> test description.
> v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
> against older libdrm as intended by v2.
>
> For: VIZ-4636
> Signed-off-by: Jeff McGee
> ---
> tests/.gitignore | 1 +
> tests/Makefile.sources | 1 +
> tests/core_getparams.c | 167
> +
> 3 files changed, 169 insertions(+)
> create mode 100644 tests/core_getparams.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 426cc67..c742308 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,6 +1,7 @@
> # Please keep sorted alphabetically
> core_get_client_auth
> core_getclient
> +core_getparams
> core_getstats
> core_getversion
> drm_import_export
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 51e8376..999c8f8 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
>
> TESTS_progs_M = \
> core_get_client_auth \
> + core_getparams \
Sorry I missed this little one: core_ is for drm core stuff shared by all
drivers (i.e. even those not supporting gem or kms). getparam is an i915
specific thing, so imo better to have drv_ as a prefix.
For our naming conventions and igt test sections see the docs at
http://people.freedesktop.org/~danvet/igt/
For review I think signing up someone from the beignet team might be best.
Cheers, Daniel
> drv_suspend \
> drv_hangman \
> gem_bad_reloc \
> diff --git a/tests/core_getparams.c b/tests/core_getparams.c
> new file mode 100644
> index 000..2855d06
> --- /dev/null
> +++ b/tests/core_getparams.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Jeff McGee
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include "drmtest.h"
> +#include "intel_chipset.h"
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Tests the export of parameters via
> DRM_IOCTL_I915_GETPARAM\n");
> +
> +int drm_fd;
> +int devid;
> +
> +static void
> +init(void)
> +{
> + drm_fd = drm_open_any();
> + devid = intel_get_drm_devid(drm_fd);
> +}
> +
> +static void
> +deinit(void)
> +{
> + close(drm_fd);
> +}
> +
> +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> +#define LOCAL_I915_PARAM_EU_TOTAL34
> +
> +static int
> +getparam(int param, int *value)
> +{
> + drm_i915_getparam_t gp;
> + int ret;
> +
> + memset(&gp, 0, sizeof(gp));
> + gp.value = value;
> + gp.param = param;
> + ret = drmIoctl(drm_fd, DRM_IOCTL_I915_GETPARAM, &gp);
> + if (ret)
> + return -errno;
> +
> + return 0;
> +}
> +
> +static void
> +subslice_total(void)
> +{
> + unsigned int subslice_total = 0;
> + int ret;
> +
> + ret = getparam(LOCAL_I915_PARAM_SUBSLICE_TOTAL, (int*)&subslice_total);
> +
> + if (ret) {
> + /*
> + * These devices are not required to implement the
> + * interface. If they do not, -ENODEV must be returned.
> + */
> + if ((intel_gen(devid) < 8) ||
> + IS_BROADWELL(devid) ||
> + igt_run_in_simulation()) {
> + igt_assert_eq(ret, -ENODEV);
> +
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On 2015-03-09 14:02, Chris Wilson wrote: On Mon, Mar 09, 2015 at 02:34:46AM +, Zou, Nanhai wrote: We don't need MAP_FIXED, we just want to avoid address 0 to be allocated. Though I think using MAP_FIXED is overkill, will bring much unnecessary complexity on both kernel and beignet side. I don't mind if people can provide stable MAP_FIXED patches to resolve this problem a few months or years later. At that time, kernel driver can revert the reserve page 0 patch. Before that reserve page 0 can benefit all the Beignet user without breaking anything. The point is that is becomes ABI. So no the kernel can't just revert it. There is nothing special about address 0 in ether GTT or virtual memory. If you require a special object allocated at address 0, allocate a special object at address 0. I've explained the ABI issue in a separate e-mail discussion, and I believe that they now fully understand what you meant. That said, their main chain of reasoning makes some sense -- there is a race condition if we rely on using MAP_FIXED, at least on systems that do not support ppgtt. Ending up in a situation where opencl applications work on other hw, but fails when run on an i915-system would, at least in my opinion, not be ideal, no matter if it's due to an unfortunate design. *If* a MAP_FIXED solution is decided upon, how can userland be sure that the GTT page mapped to 0 is indeed usable as the NULL pointer? ON a PPGTT system that would be easy enough -- it's per process, so we'll be the only process allocating a page at 0, but if allocations use a global address space that won't be possible to guarantee. I realise that the first submitted patch didn't cover the GTT case, since the first indication I got was that not only was there a special case for GTT to need page 0 for other things, but also that this was "good enough" for opencl, but it seems that a full solution would be needed. Since this is a memory area we're talking about it's not uncommon to have the 0th page represent the NULL pointer and impossible for applications to reserve, so it would hardly be an unusual and inexplicable solution. All this said, how do "the other two" (NVidia, ATI) deal with this? Implicit NULL-page, explicit MAP_FIXED allocation, or something else? Kind regards, David - Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 11:10:56AM +0200, David Weinehall wrote: > On 2015-03-09 14:02, Chris Wilson wrote: > >On Mon, Mar 09, 2015 at 02:34:46AM +, Zou, Nanhai wrote: > >>We don't need MAP_FIXED, we just want to avoid address 0 to be allocated. > >> > >>Though I think using MAP_FIXED is overkill, will bring much unnecessary > >>complexity on both kernel and beignet side. > >>I don't mind if people can provide stable MAP_FIXED patches to resolve this > >>problem a few months or years later. > >> > >>At that time, kernel driver can revert the reserve page 0 patch. > >>Before that reserve page 0 can benefit all the Beignet user without > >>breaking anything. > > > >The point is that is becomes ABI. So no the kernel can't just revert it. > >There is nothing special about address 0 in ether GTT or virtual memory. > >If you require a special object allocated at address 0, allocate a > >special object at address 0. > > I've explained the ABI issue in a separate e-mail discussion, and I > believe that they now fully understand what you meant. > > That said, their main chain of reasoning makes some sense -- there is a > race condition if we rely on using MAP_FIXED, at least on systems that > do not support ppgtt. Ending up in a situation where opencl > applications work on other hw, but fails when run on an i915-system > would, at least in my opinion, not be ideal, no matter if it's due > to an unfortunate design. > > *If* a MAP_FIXED solution is decided upon, how can userland be sure > that the GTT page mapped to 0 is indeed usable as the NULL pointer? > ON a PPGTT system that would be easy enough -- it's per process, so > we'll be the only process allocating a page at 0, but if allocations > use a global address space that won't be possible to guarantee. It's simple, userspace has no control over allocation of the GGTT. Espcially something like address 0. They can make a request to have their object at a certain location, but the kernel is entirely likely to reject their request because it is pinned to hardware. > I realise that the first submitted patch didn't cover the GTT case, > since the first indication I got was that not only was there a > special case for GTT to need page 0 for other things, but also that > this was "good enough" for opencl, but it seems that a full solution > would be needed. Exactly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/skl: Implement WaDisableHBR2
On Thu, Mar 12, 2015 at 12:03:51PM -0700, Jesse Barnes wrote: > On 03/12/2015 11:36 AM, Jani Nikula wrote: > > On Thu, 12 Mar 2015, Jesse Barnes wrote: > >> On 02/11/2015 09:43 AM, Damien Lespiau wrote: > >>> v2: Use the recently introduced INTEL_REVID() and SKL_REVID defines > >>> (Nick Hoath) > >>> > >>> Signed-off-by: Damien Lespiau > >>> --- > >>> drivers/gpu/drm/i915/intel_dp.c | 5 - > >>> 1 file changed, 4 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c > >>> b/drivers/gpu/drm/i915/intel_dp.c > >>> index d4c82d7..a7bc3e8 100644 > >>> --- a/drivers/gpu/drm/i915/intel_dp.c > >>> +++ b/drivers/gpu/drm/i915/intel_dp.c > >>> @@ -129,7 +129,10 @@ intel_dp_max_link_bw(struct intel_dp *intel_dp) > >>> case DP_LINK_BW_2_7: > >>> break; > >>> case DP_LINK_BW_5_4: /* 1.2 capable displays may advertise higher bw */ > >>> - if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || > >>> + if (IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_B0) > >>> + /* WaDisableHBR2:skl */ > >>> + max_link_bw = DP_LINK_BW_2_7; > >>> + else if (((IS_HASWELL(dev) && !IS_HSW_ULX(dev)) || > >>>INTEL_INFO(dev)->gen >= 8) && > >>> intel_dp->dpcd[DP_DPCD_REV] >= 0x12) > >>> max_link_bw = DP_LINK_BW_5_4; > >>> > >> > >> Reviewed-by: Jesse Barnes > >> Tested-by: Jesse Barnes > > > > Potentially > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89554 > > Yeah hopefully. I definitely need it here for eDP to come up reliably. Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 11:10:56AM +0200, David Weinehall wrote: > On 2015-03-09 14:02, Chris Wilson wrote: > >On Mon, Mar 09, 2015 at 02:34:46AM +, Zou, Nanhai wrote: > >>We don't need MAP_FIXED, we just want to avoid address 0 to be allocated. > >> > >>Though I think using MAP_FIXED is overkill, will bring much unnecessary > >>complexity on both kernel and beignet side. > >>I don't mind if people can provide stable MAP_FIXED patches to resolve this > >>problem a few months or years later. > >> > >>At that time, kernel driver can revert the reserve page 0 patch. > >>Before that reserve page 0 can benefit all the Beignet user without > >>breaking anything. > > > >The point is that is becomes ABI. So no the kernel can't just revert it. > >There is nothing special about address 0 in ether GTT or virtual memory. > >If you require a special object allocated at address 0, allocate a > >special object at address 0. > > I've explained the ABI issue in a separate e-mail discussion, and I believe > that they now fully understand what you meant. > > That said, their main chain of reasoning makes some sense -- there is a > race condition if we rely on using MAP_FIXED, at least on systems that > do not support ppgtt. Ending up in a situation where opencl applications > work on other hw, but fails when run on an i915-system would, at least in my > opinion, not be ideal, no matter if it's due to an unfortunate design. > > *If* a MAP_FIXED solution is decided upon, how can userland be sure that the > GTT page mapped to 0 is indeed usable as the NULL pointer? > ON a PPGTT system that would be easy enough -- it's per process, so we'll be > the only process allocating a page at 0, but if allocations > use a global address space that won't be possible to guarantee. > > I realise that the first submitted patch didn't cover the GTT case, since > the first indication I got was that not only was there a special case for > GTT to need page 0 for other things, but also that this was "good enough" > for opencl, but it seems that a full solution would be needed. > > Since this is a memory area we're talking about it's not uncommon to have > the 0th page represent the NULL pointer and impossible for applications to > reserve, so it would hardly be an unusual and inexplicable solution. > > All this said, how do "the other two" (NVidia, ATI) deal with this? > Implicit NULL-page, explicit MAP_FIXED allocation, or something else? Afaik there's no opensource ocl implementation yet that needs this. So no idea what they do. If supporting systems without full ppgtt is a requirement for you (still wonky on gen8 a bit, so might be a good strategy) then imo it's the PIN_BIAS idea I've laid out earlier in this thread. That one will work everywhere. softpin can unexpectedly fail without full ppgtt if the kernel decides to put something at a given spot, which imo means we should only expose it on full ppgtt systems. And PIN_BIAS should be fairly easy to wire up since the internal logic is all there already. So "just" needs an execbuf flag, igt test and appropriate userspace to set that new bit. > Kind regards, David > - > Intel Finland Oy > Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - > 4 Domiciled in Helsinki > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. You want to remove this when submitting to public mailing lists. Either apply for an exception or grab a 2nd mail address for mailing list communication. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Optimistically spin for the request completion
On Thu, Mar 12, 2015 at 05:32:26PM +, Tvrtko Ursulin wrote: > > On 03/12/2015 04:50 PM, Chris Wilson wrote: > >On Thu, Mar 12, 2015 at 04:41:10PM +, Tvrtko Ursulin wrote: > >>Yes I didn't mean that - but to have a boolean spinning-wait=on/off. > >>Maybe default to "on" on HZ=1000 with preemption, or the opposite, > >>something like that. > > > >I don't see the point in having the complication, until someone > >complains. In my defense, I will point to the optimistic mutex spinning > >equally having no configurable option. And since the idea is that you > >only hit this if you are abusing i915 (e.g. benchmarking, or you have a > >readback on the critical patch, or if we haven't implemented > >semaphores), I would rather fix those scenarios as they arrive rather > >than giving the user an option to break. > > I simply pointed out a theoretical potential to burn more CPU on servers. If > you know that is unlikely or only theoretical that's fine by me. > > But I'll say I was more convinced before you mentioned "until someone > complains" and "option to break". :) Server workloads on intel machines are currently transcode stuff (and there's more room in libva for better parallelism than this) and ocl (which tends to go with the busy-loops for synchronization model anyway). Also we'll stop spinning the second someone else wants the cpu, so there's really just secondary effects of the scheduler due to fewer iowait credits and more timeslices used up. I'll go with Chris and will start to care when someone complains with real benchmark numbers attached ;-) Ofc we still want broader numbers for the benchmakr benefits for this one first. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/19] drm/i915: Allocate a drm_atomic_state for the legacy modeset code
For the atomic conversion, the mode set paths need to be changed to rely
on an atomic state instead of using the staged config. By using an
atomic state for the legacy code, we will be able to convert the code
base in small chunks.
v2: Squash patch that adds state argument to intel_set_mode(). (Ander)
Make every caller of intel_set_mode() allocate state. (Daniel)
Call drm_atomic_state_clear() in set config's error path. (Daniel)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 168 +++
1 file changed, 133 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 78ea122..b61e3f6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -37,6 +37,7 @@
#include
#include "i915_drv.h"
#include "i915_trace.h"
+#include
#include
#include
#include
@@ -82,7 +83,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config);
static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *old_fb);
+ int x, int y, struct drm_framebuffer *old_fb,
+ struct drm_atomic_state *state);
static int intel_framebuffer_init(struct drm_device *dev,
struct intel_framebuffer *ifb,
struct drm_mode_fb_cmd2 *mode_cmd,
@@ -8802,6 +8804,7 @@ bool intel_get_load_detect_pipe(struct drm_connector
*connector,
struct drm_device *dev = encoder->dev;
struct drm_framebuffer *fb;
struct drm_mode_config *config = &dev->mode_config;
+ struct drm_atomic_state *state = NULL;
int ret, i = -1;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -8883,6 +8886,12 @@ retry:
old->load_detect_temp = true;
old->release_fb = NULL;
+ state = drm_atomic_state_alloc(dev);
+ if (!state)
+ return false;
+
+ state->acquire_ctx = ctx;
+
if (!mode)
mode = &load_detect_mode;
@@ -8905,7 +8914,7 @@ retry:
goto fail;
}
- if (intel_set_mode(crtc, mode, 0, 0, fb)) {
+ if (intel_set_mode(crtc, mode, 0, 0, fb, state)) {
DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
if (old->release_fb)
old->release_fb->funcs->destroy(old->release_fb);
@@ -8924,6 +8933,11 @@ retry:
else
intel_crtc->new_config = NULL;
fail_unlock:
+ if (state) {
+ drm_atomic_state_free(state);
+ state = NULL;
+ }
+
if (ret == -EDEADLK) {
drm_modeset_backoff(ctx);
goto retry;
@@ -8936,22 +8950,34 @@ void intel_release_load_detect_pipe(struct
drm_connector *connector,
struct intel_load_detect_pipe *old,
struct drm_modeset_acquire_ctx *ctx)
{
+ struct drm_device *dev = connector->dev;
struct intel_encoder *intel_encoder =
intel_attached_encoder(connector);
struct drm_encoder *encoder = &intel_encoder->base;
struct drm_crtc *crtc = encoder->crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_atomic_state *state;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
connector->base.id, connector->name,
encoder->base.id, encoder->name);
if (old->load_detect_temp) {
+ state = drm_atomic_state_alloc(dev);
+ if (!state) {
+ DRM_DEBUG_KMS("can't release load detect pipe\n");
+ return;
+ }
+
+ state->acquire_ctx = ctx;
+
to_intel_connector(connector)->new_encoder = NULL;
intel_encoder->new_crtc = NULL;
intel_crtc->new_enabled = false;
intel_crtc->new_config = NULL;
- intel_set_mode(crtc, NULL, 0, 0, NULL);
+ intel_set_mode(crtc, NULL, 0, 0, NULL, state);
+
+ drm_atomic_state_free(state);
if (old->release_fb) {
drm_framebuffer_unregister_private(old->release_fb);
@@ -10345,10 +10371,22 @@ static bool check_digital_port_conflicts(struct
drm_device *dev)
return true;
}
-static struct intel_crtc_state *
+static void
+clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
+{
+ struct drm_crtc_state tmp_state;
+
+ /* Clear only the intel specific part of the crtc state */
+ tmp_state = crtc_state->base;
+ memset(crtc_state, 0, sizeof *crtc_state);
+ crtc_state->base = tmp_state;
+}
+
+static int
intel_modeset_pipe_conf
[Intel-gfx] [PATCH v2 00/19] Remove depencies on staged config for atomic transition
Here's v2 with most of the comments from Daniel addressed. I didn't change the zeroing of the crtc_state to the duplicate state function, since it causes problems when we add the state of a crtc that isn't going through a modeset. Specifically, this would cause the code that decides whether pipes B and C can be enabled at the same time to fail, since the number of fdi lanes would be zero. The other concerns I raised with v1 are also addressed. I tested this on IVYBRIDGE using pipes B & C, and the load detect code path using Daniel's patch that adds the i915.load_detect_test parameter. Thanks, Ander Ander Conselvan de Oliveira (19): drm/i915: Add intel_atomic_get_crtc_state() helper function drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe() drm/i915: Allocate a drm_atomic_state for the legacy modeset code drm/i915: Allocate a crtc_state also when the crtc is being disabled drm/i915: Update dummy connector atomic state with current config drm/i915: Implement connector state duplication drm/i915: Copy the staged connector config to the legacy atomic state drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config() drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp() drm/i915: Don't depend on encoder->new_crtc in intel_dp_compute_config() drm/i915: Don't depend on encoder->new_crtc in intel_hdmi_compute_config drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder() drm/i915: Don't use staged config in intel_dp_mst_compute_config() drm/i915: Don't use encoder->new_crtc in intel_lvds_compute_config() drm/i915: Pass an atomic state to modeset_global_resources() functions drm/i915: Check lane sharing between pipes B & C using atomic state drm/i915: Convert intel_pipe_will_have_type() to using atomic state drm/i915: Don't look at staged config crtc when changing DRRS state drm/i915: Remove usage of encoder->new_crtc from clock computations drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/intel_crt.c | 3 +- drivers/gpu/drm/i915/intel_ddi.c | 24 +- drivers/gpu/drm/i915/intel_display.c | 544 ++- drivers/gpu/drm/i915/intel_dp.c | 5 +- drivers/gpu/drm/i915/intel_dp_mst.c | 18 +- drivers/gpu/drm/i915/intel_drv.h | 16 +- drivers/gpu/drm/i915/intel_dsi.c | 1 + drivers/gpu/drm/i915/intel_dvo.c | 1 + drivers/gpu/drm/i915/intel_hdmi.c| 22 +- drivers/gpu/drm/i915/intel_lvds.c| 3 +- drivers/gpu/drm/i915/intel_sdvo.c| 1 + drivers/gpu/drm/i915/intel_tv.c | 3 +- 13 files changed, 484 insertions(+), 161 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/19] drm/i915: Copy the staged connector config to the legacy atomic state
With this in place, we can start converting pieces of the modeset code
to look at the connector atomic state instead of the staged config.
v2: Handle the load detect staged config changes too. (Ander)
Remove unnecessary blank line. (Daniel)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 52 +++-
1 file changed, 45 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index abdbd0c..6465f6d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8805,6 +8805,7 @@ bool intel_get_load_detect_pipe(struct drm_connector
*connector,
struct drm_framebuffer *fb;
struct drm_mode_config *config = &dev->mode_config;
struct drm_atomic_state *state = NULL;
+ struct drm_connector_state *connector_state;
int ret, i = -1;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -8892,6 +8893,15 @@ retry:
state->acquire_ctx = ctx;
+ connector_state = drm_atomic_get_connector_state(state, connector);
+ if (IS_ERR(connector_state)) {
+ ret = PTR_ERR(connector_state);
+ goto fail;
+ }
+
+ connector_state->crtc = crtc;
+ connector_state->best_encoder = &intel_encoder->base;
+
if (!mode)
mode = &load_detect_mode;
@@ -8957,6 +8967,7 @@ void intel_release_load_detect_pipe(struct drm_connector
*connector,
struct drm_crtc *crtc = encoder->crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_atomic_state *state;
+ struct drm_connector_state *connector_state;
DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
connector->base.id, connector->name,
@@ -8964,17 +8975,23 @@ void intel_release_load_detect_pipe(struct
drm_connector *connector,
if (old->load_detect_temp) {
state = drm_atomic_state_alloc(dev);
- if (!state) {
- DRM_DEBUG_KMS("can't release load detect pipe\n");
- return;
- }
+ if (!state)
+ goto fail;
state->acquire_ctx = ctx;
+ connector_state = drm_atomic_get_connector_state(state,
connector);
+ if (IS_ERR(connector_state))
+ goto fail;
+
to_intel_connector(connector)->new_encoder = NULL;
intel_encoder->new_crtc = NULL;
intel_crtc->new_enabled = false;
intel_crtc->new_config = NULL;
+
+ connector_state->best_encoder = NULL;
+ connector_state->crtc = NULL;
+
intel_set_mode(crtc, NULL, 0, 0, NULL, state);
drm_atomic_state_free(state);
@@ -8990,6 +9007,11 @@ void intel_release_load_detect_pipe(struct drm_connector
*connector,
/* Switch crtc and encoder back off if necessary */
if (old->dpms_mode != DRM_MODE_DPMS_ON)
connector->funcs->dpms(connector, old->dpms_mode);
+
+ return;
+fail:
+ DRM_DEBUG_KMS("Couldn't release load detect pipe.\n");
+ drm_atomic_state_free(state);
}
static int i9xx_pll_refclk(struct drm_device *dev,
@@ -11646,9 +11668,11 @@ intel_set_config_compute_mode_changes(struct
drm_mode_set *set,
static int
intel_modeset_stage_output_state(struct drm_device *dev,
struct drm_mode_set *set,
-struct intel_set_config *config)
+struct intel_set_config *config,
+struct drm_atomic_state *state)
{
struct intel_connector *connector;
+ struct drm_connector_state *connector_state;
struct intel_encoder *encoder;
struct intel_crtc *crtc;
int ro;
@@ -11712,6 +11736,14 @@ intel_modeset_stage_output_state(struct drm_device
*dev,
}
connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
+ connector_state =
+ drm_atomic_get_connector_state(state, &connector->base);
+ if (IS_ERR(connector_state))
+ return PTR_ERR(connector_state);
+
+ connector_state->crtc = new_crtc;
+ connector_state->best_encoder = &connector->new_encoder->base;
+
DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
connector->base.base.id,
connector->base.name,
@@ -11744,9 +11776,15 @@ intel_modeset_stage_output_state(struct drm_device
*dev,
}
/* Now we've also updated encoder->new_crtc for all encoders. */
for_each_intel_connector(dev, connector) {
- if (connector->new_encoder)
+ connector_state =
+ drm_atomic_get_connector_state(state
[Intel-gfx] [PATCH 06/19] drm/i915: Implement connector state duplication
So that we can add connector states to the drm_atomic_state used in the
legacy modeset.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_crt.c| 1 +
drivers/gpu/drm/i915/intel_dp.c | 1 +
drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
drivers/gpu/drm/i915/intel_dsi.c| 1 +
drivers/gpu/drm/i915/intel_dvo.c| 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 1 +
drivers/gpu/drm/i915/intel_lvds.c | 1 +
drivers/gpu/drm/i915/intel_sdvo.c | 1 +
drivers/gpu/drm/i915/intel_tv.c | 1 +
9 files changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 974534e..573aaff 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -794,6 +794,7 @@ static const struct drm_connector_funcs
intel_crt_connector_funcs = {
.destroy = intel_crt_destroy,
.set_property = intel_crt_set_property,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_get_property = intel_connector_atomic_get_property,
};
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33d5877..92bfbe6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4545,6 +4545,7 @@ static const struct drm_connector_funcs
intel_dp_connector_funcs = {
.atomic_get_property = intel_connector_atomic_get_property,
.destroy = intel_dp_connector_destroy,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs
= {
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index be12492..5c06a06 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -317,6 +317,7 @@ static const struct drm_connector_funcs
intel_dp_mst_connector_funcs = {
.atomic_get_property = intel_connector_atomic_get_property,
.destroy = intel_dp_mst_connector_destroy,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
static int intel_dp_mst_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c8c8b24..572251e 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -975,6 +975,7 @@ static const struct drm_connector_funcs
intel_dsi_connector_funcs = {
.fill_modes = drm_helper_probe_single_connector_modes,
.atomic_get_property = intel_connector_atomic_get_property,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
void intel_dsi_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index d857951..4ccd6c3 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -393,6 +393,7 @@ static const struct drm_connector_funcs
intel_dvo_connector_funcs = {
.fill_modes = drm_helper_probe_single_connector_modes,
.atomic_get_property = intel_connector_atomic_get_property,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
static const struct drm_connector_helper_funcs
intel_dvo_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 995c5b2..b13a8be 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1618,6 +1618,7 @@ static const struct drm_connector_funcs
intel_hdmi_connector_funcs = {
.atomic_get_property = intel_connector_atomic_get_property,
.destroy = intel_hdmi_destroy,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
static const struct drm_connector_helper_funcs
intel_hdmi_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/i915/intel_lvds.c
b/drivers/gpu/drm/i915/intel_lvds.c
index 24e8730..2b008b02 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -535,6 +535,7 @@ static const struct drm_connector_funcs
intel_lvds_connector_funcs = {
.atomic_get_property = intel_connector_atomic_get_property,
.destroy = intel_lvds_destroy,
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
};
static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
diff --git a/drivers/gpu/drm/i91
[Intel-gfx] [PATCH 02/19] drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe()
For now this is not necessary since intel_set_mode() doesn't acquire any
new locks. However, once that function is converted to atomic, that will
change, since we'll pass an atomic state to it, and that needs to have
the right acquire context set.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_crt.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 5 +++--
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_tv.c | 2 +-
4 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e66e17a..974534e 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -708,7 +708,7 @@ intel_crt_detect(struct drm_connector *connector, bool
force)
status = connector_status_connected;
else
status = intel_crt_load_detect(crt);
- intel_release_load_detect_pipe(connector, &tmp);
+ intel_release_load_detect_pipe(connector, &tmp, &ctx);
} else
status = connector_status_unknown;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2d3c9d8..78ea122 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8933,7 +8933,8 @@ fail_unlock:
}
void intel_release_load_detect_pipe(struct drm_connector *connector,
- struct intel_load_detect_pipe *old)
+ struct intel_load_detect_pipe *old,
+ struct drm_modeset_acquire_ctx *ctx)
{
struct intel_encoder *intel_encoder =
intel_attached_encoder(connector);
@@ -13479,7 +13480,7 @@ static void intel_enable_pipe_a(struct drm_device *dev)
return;
if (intel_get_load_detect_pipe(crt, NULL, &load_detect_temp, ctx))
- intel_release_load_detect_pipe(crt, &load_detect_temp);
+ intel_release_load_detect_pipe(crt, &load_detect_temp, ctx);
}
static bool
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5bae4aa..ec99046 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -958,7 +958,8 @@ bool intel_get_load_detect_pipe(struct drm_connector
*connector,
struct intel_load_detect_pipe *old,
struct drm_modeset_acquire_ctx *ctx);
void intel_release_load_detect_pipe(struct drm_connector *connector,
- struct intel_load_detect_pipe *old);
+ struct intel_load_detect_pipe *old,
+ struct drm_modeset_acquire_ctx *ctx);
int intel_pin_and_fence_fb_obj(struct drm_plane *plane,
struct drm_framebuffer *fb,
struct intel_engine_cs *pipelined);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 892d23c..4e6684e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1332,7 +1332,7 @@ intel_tv_detect(struct drm_connector *connector, bool
force)
if (intel_get_load_detect_pipe(connector, &mode, &tmp, &ctx)) {
type = intel_tv_detect_type(intel_tv, connector);
- intel_release_load_detect_pipe(connector, &tmp);
+ intel_release_load_detect_pipe(connector, &tmp, &ctx);
status = type < 0 ?
connector_status_disconnected :
connector_status_connected;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/19] drm/i915: Remove usage of encoder->new_crtc from clock computations
Some of the crtc_compute_clock() still depended on encoder->new_crtc
since they didn't use intel_pipe_will_have_type() and used an open
coded version of that function instead. This patch replaces those with
the appropriate code that checks the atomic state intead.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 45 +---
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index be1979e..35ed651 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6696,11 +6696,18 @@ static int i9xx_crtc_compute_clock(struct intel_crtc
*crtc,
bool is_lvds = false, is_dsi = false;
struct intel_encoder *encoder;
const intel_limit_t *limit;
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct drm_connector_state *connector_state;
+ int i;
- for_each_intel_encoder(dev, encoder) {
- if (encoder->new_crtc != crtc)
+ for (i = 0; i < state->num_connector; i++) {
+ connector_state = state->connector_states[i];
+ if (!state->connectors[i] ||
+ connector_state->crtc != &crtc->base)
continue;
+ encoder = to_intel_encoder(connector_state->best_encoder);
+
switch (encoder->type) {
case INTEL_OUTPUT_LVDS:
is_lvds = true;
@@ -7374,18 +7381,24 @@ void intel_init_pch_refclk(struct drm_device *dev)
lpt_init_pch_refclk(dev);
}
-static int ironlake_get_refclk(struct drm_crtc *crtc)
+static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc->dev;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct drm_connector_state *connector_state;
struct intel_encoder *encoder;
- int num_connectors = 0;
+ int num_connectors = 0, i;
bool is_lvds = false;
- for_each_intel_encoder(dev, encoder) {
- if (encoder->new_crtc != to_intel_crtc(crtc))
+ for (i = 0; i < state->num_connector; i++) {
+ connector_state = state->connector_states[i];
+ if (!state->connectors[i] ||
+ connector_state->crtc != crtc_state->base.crtc)
continue;
+ encoder = to_intel_encoder(connector_state->best_encoder);
+
switch (encoder->type) {
case INTEL_OUTPUT_LVDS:
is_lvds = true;
@@ -7578,7 +7591,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
- refclk = ironlake_get_refclk(crtc);
+ refclk = ironlake_get_refclk(crtc_state);
/*
* Returns a set of divisors for the desired target clock with the given
@@ -7633,16 +7646,22 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc
*intel_crtc,
struct drm_crtc *crtc = &intel_crtc->base;
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct intel_encoder *intel_encoder;
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct drm_connector_state *connector_state;
+ struct intel_encoder *encoder;
uint32_t dpll;
- int factor, num_connectors = 0;
+ int factor, num_connectors = 0, i;
bool is_lvds = false, is_sdvo = false;
- for_each_intel_encoder(dev, intel_encoder) {
- if (intel_encoder->new_crtc != to_intel_crtc(crtc))
+ for (i = 0; i < state->num_connector; i++) {
+ connector_state = state->connector_states[i];
+ if (!state->connectors[i] ||
+ connector_state->crtc != crtc_state->base.crtc)
continue;
- switch (intel_encoder->type) {
+ encoder = to_intel_encoder(connector_state->best_encoder);
+
+ switch (encoder->type) {
case INTEL_OUTPUT_LVDS:
is_lvds = true;
break;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 12/19] drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder()
Instead of using connector->new_encoder, get the same information from
the pipe_config, thus making the function ready for the atomic
conversion.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_ddi.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8aee7d7..47b9307 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -492,17 +492,23 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
}
static struct intel_encoder *
-intel_ddi_get_crtc_new_encoder(struct intel_crtc *crtc)
+intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc->base.dev;
- struct intel_encoder *intel_encoder, *ret = NULL;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct intel_encoder *ret = NULL;
+ struct drm_atomic_state *state;
int num_encoders = 0;
+ int i;
- for_each_intel_encoder(dev, intel_encoder) {
- if (intel_encoder->new_crtc == crtc) {
- ret = intel_encoder;
- num_encoders++;
- }
+ state = crtc_state->base.state;
+
+ for (i = 0; i < state->num_connector; i++) {
+ if (!state->connectors[i] ||
+ state->connector_states[i]->crtc != crtc_state->base.crtc)
+ continue;
+
+ ret =
to_intel_encoder(state->connector_states[i]->best_encoder);
+ num_encoders++;
}
WARN(num_encoders != 1, "%d encoders on crtc for pipe %c\n",
num_encoders,
@@ -1216,7 +1222,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
{
struct drm_device *dev = intel_crtc->base.dev;
struct intel_encoder *intel_encoder =
- intel_ddi_get_crtc_new_encoder(intel_crtc);
+ intel_ddi_get_crtc_new_encoder(crtc_state);
int clock = crtc_state->port_clock;
if (IS_SKYLAKE(dev))
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/19] drm/i915: Add intel_atomic_get_crtc_state() helper function
The pattern of getting the crtc state with drm_atomic_get_crtc_state()
and then converting it to intel_crtc_state will repeat quite often in
the following patches, so add a helper function to save some typing.
v2: Fix upcasting so that crtc_state base field could be moved. (Daniel)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_drv.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c77128c..5bae4aa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -35,6 +35,7 @@
#include
#include
#include
+#include
#define DIV_ROUND_CLOSEST_ULL(ll, d) \
({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
@@ -564,6 +565,7 @@ struct cxsr_latency {
};
#define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
+#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
#define to_intel_connector(x) container_of(x, struct intel_connector, base)
#define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
#define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base)
@@ -1282,6 +1284,17 @@ int intel_connector_atomic_get_property(struct
drm_connector *connector,
struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
void intel_crtc_destroy_state(struct drm_crtc *crtc,
struct drm_crtc_state *state);
+static inline struct intel_crtc_state *
+intel_atomic_get_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
+{
+ struct drm_crtc_state *crtc_state;
+ crtc_state = drm_atomic_get_crtc_state(state, &crtc->base);
+ if (IS_ERR(crtc_state))
+ return ERR_PTR(PTR_ERR(crtc_state));
+
+ return to_intel_crtc_state(crtc_state);
+}
/* intel_atomic_plane.c */
struct intel_plane_state *intel_create_plane_state(struct drm_plane *plane);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/19] drm/i915: Don't use encoder->new_crtc in compute_baseline_pipe_bpp()
Move towards atomic by using the legacy modeset's drm_atomic_state
instead.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 1609628..f0ae907 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10216,8 +10216,9 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config)
{
struct drm_device *dev = crtc->base.dev;
+ struct drm_atomic_state *state;
struct intel_connector *connector;
- int bpp;
+ int bpp, i;
switch (fb->pixel_format) {
case DRM_FORMAT_C8:
@@ -10257,10 +10258,13 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
pipe_config->pipe_bpp = bpp;
+ state = pipe_config->base.state;
+
/* Clamp display bpp to EDID value */
- for_each_intel_connector(dev, connector) {
- if (!connector->new_encoder ||
- connector->new_encoder->new_crtc != crtc)
+ for (i = 0; i < state->num_connector; i++) {
+ connector = to_intel_connector(state->connectors[i]);
+ if (!connector ||
+ state->connector_states[i]->crtc != &crtc->base)
continue;
connected_sink_compute_bpp(connector, pipe_config);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/19] drm/i915: Allocate a crtc_state also when the crtc is being disabled
For consistency, allocate a new crtc_state for a crtc that is being
disabled. Previously only the enabled value of the current state would
change.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 36 +---
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b61e3f6..62b9021 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11188,14 +11188,21 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
unsigned *prepare_pipes,
unsigned *disable_pipes)
{
+ struct drm_device *dev = crtc->dev;
struct intel_crtc_state *pipe_config = NULL;
+ struct intel_crtc *intel_crtc;
int ret = 0;
intel_modeset_affected_pipes(crtc, modeset_pipes,
prepare_pipes, disable_pipes);
- if ((*modeset_pipes) == 0)
- return NULL;
+ for_each_intel_crtc_masked(dev, *disable_pipes, intel_crtc) {
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config))
+ return pipe_config;
+
+ pipe_config->base.enable = false;
+ }
/*
* Note this needs changes when we start tracking multiple modes
@@ -11203,18 +11210,25 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
* (i.e. one pipe_config for each crtc) rather than just the one
* for this crtc.
*/
- ret = intel_modeset_pipe_config(crtc, fb, mode, state);
- if (ret)
- return ERR_PTR(ret);
+ for_each_intel_crtc_masked(dev, *modeset_pipes, intel_crtc) {
+ /* FIXME: For now we still expect modeset_pipes has at most
+* one bit set. */
+ if (WARN_ON(&intel_crtc->base != crtc))
+ continue;
- pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
- if (IS_ERR(pipe_config))
- return pipe_config;
+ ret = intel_modeset_pipe_config(crtc, fb, mode, state);
+ if (ret)
+ return ERR_PTR(ret);
+
+ pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+ if (IS_ERR(pipe_config))
+ return pipe_config;
- intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
- "[modeset]");
+ intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+ "[modeset]");
+ }
- return pipe_config;
+ return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
}
static int __intel_set_mode_setup_plls(struct drm_device *dev,
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 16/19] drm/i915: Check lane sharing between pipes B & C using atomic state
Makes that code atomic ready.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 49 ++--
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index e720a48..8c97186 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5537,13 +5537,20 @@ bool intel_connector_get_hw_state(struct
intel_connector *connector)
return encoder->get_hw_state(encoder, &pipe);
}
-static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
+static int pipe_required_fdi_lanes(struct drm_atomic_state *state,
+ enum pipe pipe)
{
struct intel_crtc *crtc =
- to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+ to_intel_crtc(intel_get_crtc_for_pipe(state->dev, pipe));
+ struct intel_crtc_state *crtc_state;
+
+ crtc_state = intel_atomic_get_crtc_state(state, crtc);
+ if (WARN_ON(IS_ERR(crtc_state))) {
+ /* Cause modeset to fail due to excess lanes. */
+ return 5;
+ }
- if (crtc->base.state->enable &&
- crtc->config->has_pch_encoder)
+ if (crtc_state->base.enable && crtc_state->has_pch_encoder)
return crtc->config->fdi_lanes;
return 0;
@@ -5552,6 +5559,8 @@ static int pipe_required_fdi_lanes(struct drm_device
*dev, enum pipe pipe)
static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe,
struct intel_crtc_state *pipe_config)
{
+ struct drm_atomic_state *state = pipe_config->base.state;
+
DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
pipe_name(pipe), pipe_config->fdi_lanes);
if (pipe_config->fdi_lanes > 4) {
@@ -5579,7 +5588,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device
*dev, enum pipe pipe,
return true;
case PIPE_B:
if (pipe_config->fdi_lanes > 2 &&
- pipe_required_fdi_lanes(dev, PIPE_C) > 0) {
+ pipe_required_fdi_lanes(state, PIPE_C) > 0) {
DRM_DEBUG_KMS("invalid shared fdi lane config on pipe
%c: %i lanes\n",
pipe_name(pipe), pipe_config->fdi_lanes);
return false;
@@ -5591,7 +5600,7 @@ static bool ironlake_check_fdi_lanes(struct drm_device
*dev, enum pipe pipe,
pipe_name(pipe), pipe_config->fdi_lanes);
return false;
}
- if (pipe_required_fdi_lanes(dev, PIPE_B) > 2) {
+ if (pipe_required_fdi_lanes(state, PIPE_B) > 2) {
DRM_DEBUG_KMS("fdi link B uses too many lanes to enable
link C\n");
return false;
}
@@ -5601,15 +5610,41 @@ static bool ironlake_check_fdi_lanes(struct drm_device
*dev, enum pipe pipe,
}
}
+static int add_pipe_b_c_to_state(struct drm_atomic_state *state)
+{
+ struct intel_crtc *pipe_B =
+ to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_B));
+ struct intel_crtc *pipe_C =
+ to_intel_crtc(intel_get_crtc_for_pipe(state->dev, PIPE_C));
+ struct intel_crtc_state *crtc_state;
+
+ crtc_state = intel_atomic_get_crtc_state(state, pipe_B);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ crtc_state = intel_atomic_get_crtc_state(state, pipe_C);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ return 0;
+}
+
#define RETRY 1
static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
struct intel_crtc_state *pipe_config)
{
struct drm_device *dev = intel_crtc->base.dev;
struct drm_display_mode *adjusted_mode =
&pipe_config->base.adjusted_mode;
- int lane, link_bw, fdi_dotclock;
+ int lane, link_bw, fdi_dotclock, ret;
bool setup_ok, needs_recompute = false;
+ if (IS_IVYBRIDGE(dev) &&
+ (intel_crtc->pipe == PIPE_B || intel_crtc->pipe == PIPE_C)) {
+ ret = add_pipe_b_c_to_state(pipe_config->base.state);
+ if (ret < 0)
+ return ret;
+ }
+
retry:
/* FDI is a binary signal running at ~2.7GHz, encoding
* each output octet as 10 bits. The actual frequency
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/19] drm/i915: Don't use staged config in intel_dp_mst_compute_config()
Move towards atomic by using the legacy modeset's drm_atomic_state
instead.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_dp_mst.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 5c06a06..b132fe6 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -36,11 +36,11 @@ static bool intel_dp_mst_compute_config(struct
intel_encoder *encoder,
struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
struct intel_digital_port *intel_dig_port = intel_mst->primary;
struct intel_dp *intel_dp = &intel_dig_port->dp;
- struct drm_device *dev = encoder->base.dev;
- int bpp;
+ struct drm_atomic_state *state;
+ int bpp, i;
int lane_count, slots;
struct drm_display_mode *adjusted_mode =
&pipe_config->base.adjusted_mode;
- struct intel_connector *found = NULL, *intel_connector;
+ struct intel_connector *found = NULL;
int mst_pbn;
pipe_config->dp_encoder_is_mst = true;
@@ -58,9 +58,14 @@ static bool intel_dp_mst_compute_config(struct intel_encoder
*encoder,
pipe_config->pipe_bpp = 24;
pipe_config->port_clock =
drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
- for_each_intel_connector(dev, intel_connector) {
- if (intel_connector->new_encoder == encoder) {
- found = intel_connector;
+ state = pipe_config->base.state;
+
+ for (i = 0; i < state->num_connector; i++) {
+ if (!state->connectors[i])
+ continue;
+
+ if (state->connector_states[i]->best_encoder == &encoder->base)
{
+ found = to_intel_connector(state->connectors[i]);
break;
}
}
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/19] drm/i915: Don't depend on encoder->new_crtc in intel_hdmi_compute_config
Move towards atomic by using the legacy modeset's drm_atomic_state
instead.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_hdmi.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index b13a8be..cacbafd 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -951,19 +951,30 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
return MODE_OK;
}
-static bool hdmi_12bpc_possible(struct intel_crtc *crtc)
+static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc->base.dev;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
+ struct drm_atomic_state *state;
struct intel_encoder *encoder;
+ struct drm_connector_state *connector_state;
int count = 0, count_hdmi = 0;
+ int i;
if (HAS_GMCH_DISPLAY(dev))
return false;
- for_each_intel_encoder(dev, encoder) {
- if (encoder->new_crtc != crtc)
+ state = crtc_state->base.state;
+
+ for (i = 0; i < state->num_connector; i++) {
+ if (!state->connectors[i])
continue;
+ connector_state = state->connector_states[i];
+ if (connector_state->crtc != crtc_state->base.crtc)
+ continue;
+
+ encoder = to_intel_encoder(connector_state->best_encoder);
+
count_hdmi += encoder->type == INTEL_OUTPUT_HDMI;
count++;
}
@@ -1020,7 +1031,7 @@ bool intel_hdmi_compute_config(struct intel_encoder
*encoder,
*/
if (pipe_config->pipe_bpp > 8*3 && pipe_config->has_hdmi_sink &&
clock_12bpc <= portclock_limit &&
- hdmi_12bpc_possible(encoder->new_crtc)) {
+ hdmi_12bpc_possible(pipe_config)) {
DRM_DEBUG_KMS("picking bpc to 12 for HDMI output\n");
desired_bpp = 12*3;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/19] drm/i915: Update dummy connector atomic state with current config
Keep that state updated so that we can write code that depends on it on
the follow up patches.
v2: Fix BUG() due to stale connector_state->crtc value. (Chandra)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 41
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 62b9021..abdbd0c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10114,6 +10114,27 @@ static void
intel_modeset_update_staged_output_state(struct drm_device *dev)
}
}
+/* Transitional helper to copy current connector/encoder state to
+ * connector->state. This is needed so that code that is partially
+ * converted to atomic does the right thing.
+ */
+static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
+{
+ struct intel_connector *connector;
+
+ for_each_intel_connector(dev, connector) {
+ if (connector->base.encoder) {
+ connector->base.state->best_encoder =
+ connector->base.encoder;
+ connector->base.state->crtc =
+ connector->base.encoder->crtc;
+ } else {
+ connector->base.state->best_encoder = NULL;
+ connector->base.state->crtc = NULL;
+ }
+ }
+}
+
/**
* intel_modeset_commit_output_state
*
@@ -10137,6 +10158,8 @@ static void intel_modeset_commit_output_state(struct
drm_device *dev)
crtc->base.state->enable = crtc->new_enabled;
crtc->base.enabled = crtc->new_enabled;
}
+
+ intel_modeset_update_connector_atomic_state(dev);
}
static void
@@ -12876,15 +12899,13 @@ static void intel_setup_outputs(struct drm_device
*dev)
* be removed since we'll be setting up real connector state, which
* will contain Intel-specific properties.
*/
- if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
- list_for_each_entry(connector,
- &dev->mode_config.connector_list,
- head) {
- if (!WARN_ON(connector->state)) {
- connector->state =
- kzalloc(sizeof(*connector->state),
- GFP_KERNEL);
- }
+ /* FIXME: need to update the comment above. */
+ list_for_each_entry(connector,
+ &dev->mode_config.connector_list,
+ head) {
+ if (!WARN_ON(connector->state)) {
+ connector->state = kzalloc(sizeof(*connector->state),
+ GFP_KERNEL);
}
}
@@ -13937,6 +13958,8 @@ void intel_modeset_setup_hw_state(struct drm_device
*dev,
"[setup_hw_state]");
}
+ intel_modeset_update_connector_atomic_state(dev);
+
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 17/19] drm/i915: Convert intel_pipe_will_have_type() to using atomic state
Pass a crtc_state to it and find whether the pipe has an encoder of a
given type by looking at the drm_atomic_state the crtc_state points to.
Until recently i9xx_get_refclk() used to be called indirectly from
vlv_force_pll_on() with a dummy crtc_state. That dummy crtc state is not
converted to be part of a full drm atomic state, so add a WARN in case
someone decides to call that again with such a dummy state.
v2: Warn if there is no connectors for a given crtc. (Daniel)
Replace comment i9xx_get_refclk() with a WARN_ON(). (Ander)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 133 +--
2 files changed, 83 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 490ab8c..576e101 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
* Returns true on success, false on failure.
*/
bool (*find_dpll)(const struct intel_limit *limit,
- struct intel_crtc *crtc,
+ struct intel_crtc_state *crtc_state,
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 8c97186..be1979e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -431,25 +431,41 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum
intel_output_type type)
* intel_pipe_has_type() but looking at encoder->new_crtc instead of
* encoder->crtc.
*/
-static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
+static bool intel_pipe_will_have_type(const struct intel_crtc_state
*crtc_state,
+ int type)
{
- struct drm_device *dev = crtc->base.dev;
+ struct drm_atomic_state *state = crtc_state->base.state;
+ struct drm_connector_state *connector_state;
struct intel_encoder *encoder;
+ int i, num_connectors = 0;
+
+ for (i = 0; i < state->num_connector; i++) {
+ if (!state->connectors[i])
+ continue;
+
+ connector_state = state->connector_states[i];
+ if (connector_state->crtc != crtc_state->base.crtc)
+ continue;
+
+ num_connectors++;
- for_each_intel_encoder(dev, encoder)
- if (encoder->new_crtc == crtc && encoder->type == type)
+ encoder = to_intel_encoder(connector_state->best_encoder);
+ if (encoder->type == type)
return true;
+ }
+
+ WARN_ON(num_connectors == 0);
return false;
}
-static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
- int refclk)
+static const intel_limit_t *
+intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
{
- struct drm_device *dev = crtc->base.dev;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
const intel_limit_t *limit;
- if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+ if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
if (intel_is_dual_link_lvds(dev)) {
if (refclk == 10)
limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -467,20 +483,21 @@ static const intel_limit_t *intel_ironlake_limit(struct
intel_crtc *crtc,
return limit;
}
-static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
+static const intel_limit_t *
+intel_g4x_limit(struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc->base.dev;
+ struct drm_device *dev = crtc_state->base.crtc->dev;
const intel_limit_t *limit;
- if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
+ if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
if (intel_is_dual_link_lvds(dev))
limit = &intel_limits_g4x_dual_channel_lvds;
else
limit = &intel_limits_g4x_single_channel_lvds;
- } else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
- intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
+ } else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
+ intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
limit = &intel_limits_g4x_hdmi;
- } else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
+ } else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
limit = &intel_limits_g4x_sdvo;
} else /* The op
[Intel-gfx] [PATCH 18/19] drm/i915: Don't look at staged config crtc when changing DRRS state
The function intel_dp_set_drrs_state() would decide which pipe to
downclock based on the staged config for the given connector. However,
the result of that function is immediate, and it uses input values from
crtc->config, so it should be looking at the current crtc instead.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 510927e..7954302 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4932,7 +4932,7 @@ static void intel_dp_set_drrs_state(struct drm_device
*dev, int refresh_rate)
dig_port = dp_to_dig_port(intel_dp);
encoder = &dig_port->base;
- intel_crtc = encoder->new_crtc;
+ intel_crtc = to_intel_crtc(encoder->base.crtc);
if (!intel_crtc) {
DRM_DEBUG_KMS("DRRS: intel_crtc not initialized\n");
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/19] drm/i915: Don't use encoder->new_crtc in intel_modeset_pipe_config()
Move towards atomic by using the legacy modeset's drm_atomic_state
instead.
v2: Move call to drm_atomic_add_affected_connectors() to
intel_modeset_compute_config(). (Daniel)
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 6465f6d..1609628 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10435,8 +10435,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
{
struct drm_device *dev = crtc->dev;
struct intel_encoder *encoder;
+ struct intel_connector *connector;
+ struct drm_connector_state *connector_state;
struct intel_crtc_state *pipe_config;
int plane_bpp, ret = -EINVAL;
+ int i;
bool retry = true;
if (!check_encoder_cloning(to_intel_crtc(crtc))) {
@@ -10510,11 +10513,17 @@ encoder_retry:
* adjust it according to limitations or connector properties, and also
* a chance to reject the mode entirely.
*/
- for_each_intel_encoder(dev, encoder) {
+ for (i = 0; i < state->num_connector; i++) {
+ connector = to_intel_connector(state->connectors[i]);
+ if (!connector)
+ continue;
- if (&encoder->new_crtc->base != crtc)
+ connector_state = state->connector_states[i];
+ if (connector_state->crtc != crtc)
continue;
+ encoder = to_intel_encoder(connector_state->best_encoder);
+
if (!(encoder->compute_config(encoder, pipe_config))) {
DRM_DEBUG_KMS("Encoder config failure\n");
goto fail;
@@ -11238,6 +11247,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
struct intel_crtc *intel_crtc;
int ret = 0;
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ return ERR_PTR(ret);
+
intel_modeset_affected_pipes(crtc, modeset_pipes,
prepare_pipes, disable_pipes);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 15/19] drm/i915: Pass an atomic state to modeset_global_resources() functions
Follow up patches will convert some functions called from there to use
the atomic state, instead of directly accessing the new or current
config. This patch just changes the parameters, but shouldn't have any
functional changes.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 10 ++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a80b15f..490ab8c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -550,7 +550,7 @@ struct drm_i915_display_funcs {
struct drm_crtc *crtc,
uint32_t sprite_width, uint32_t sprite_height,
int pixel_size, bool enable, bool scaled);
- void (*modeset_global_resources)(struct drm_device *dev);
+ void (*modeset_global_resources)(struct drm_atomic_state *state);
/* Returns the active state of the crtc, and if the crtc is active,
* fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index f0ae907..e720a48 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4826,8 +4826,9 @@ static unsigned long get_crtc_power_domains(struct
drm_crtc *crtc)
return mask;
}
-static void modeset_update_crtc_power_domains(struct drm_device *dev)
+static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
{
+ struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long pipe_domains[I915_MAX_PIPES] = { 0, };
struct intel_crtc *crtc;
@@ -4849,7 +4850,7 @@ static void modeset_update_crtc_power_domains(struct
drm_device *dev)
}
if (dev_priv->display.modeset_global_resources)
- dev_priv->display.modeset_global_resources(dev);
+ dev_priv->display.modeset_global_resources(state);
for_each_intel_crtc(dev, crtc) {
enum intel_display_power_domain domain;
@@ -5097,8 +5098,9 @@ static void vlv_program_pfi_credits(struct
drm_i915_private *dev_priv)
WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
}
-static void valleyview_modeset_global_resources(struct drm_device *dev)
+static void valleyview_modeset_global_resources(struct drm_atomic_state *state)
{
+ struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int max_pixclk = intel_mode_max_pixclk(dev_priv);
int req_cdclk = valleyview_calc_cdclk(dev_priv, max_pixclk);
@@ -11405,7 +11407,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* update the the output configuration. */
intel_modeset_update_state(dev, prepare_pipes);
- modeset_update_crtc_power_domains(dev);
+ modeset_update_crtc_power_domains(pipe_config->base.state);
/* Set up the DPLL and any encoders state that needs to adjust or depend
* on the DPLL.
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/19] drm/i915: Don't use encoder->new_crtc in intel_lvds_compute_config()
Move towards atomic by using the legacy modeset's drm_atomic_state instead. Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_lvds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 2b008b02..06d2da3 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -286,7 +286,7 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, struct intel_connector *intel_connector = &lvds_encoder->attached_connector->base; struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; - struct intel_crtc *intel_crtc = lvds_encoder->base.new_crtc; + struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); unsigned int lvds_bpp; /* Should never happen!! */ -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/19] drm/i915: Don't depend on encoder->new_crtc in intel_dp_compute_config()
Move towards atomic by using the legacy modeset's drm_atomic_state instead. Signed-off-by: Ander Conselvan de Oliveira --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 92bfbe6..510927e 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1265,7 +1265,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); enum port port = dp_to_dig_port(intel_dp)->port; - struct intel_crtc *intel_crtc = encoder->new_crtc; + struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); struct intel_connector *intel_connector = intel_dp->attached_connector; int lane_count, clock; int min_lane_count = 1; -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
My only concern is about the following macros:
> +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> +#define LOCAL_I915_PARAM_EU_TOTAL34
How about to just use the definitons in the kernel header file?
For an example:
#include
#ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
//Put all the code into this block.
#endif
Then we can avoid put the same definitions in different files,
and we can avoid unecessary testing on an old kernel which doesn't
have this kernel interface.
For all the other part, it LGTM.
Reviewed-by: Zhigang Gong
Thanks,
Zhigang Gong.
On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mc...@intel.com wrote:
> From: Jeff McGee
>
> New test core_getparams consists of 2 subtests, each one testing
> the ability of userspace to query the correct value of a GT config
> attribute: subslice total or EU total. drm/i915 implementation of
> these queries is required for Cherryview and Gen9+ devices (non-
> simulated).
>
> v2: Duplicate small amount of new libdrm functionality to avoid
> bumping libdrm version requirement (Daniel). Convert some
> igt_asserts to the appropriate comparison variants. Add a
> test description.
> v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
> against older libdrm as intended by v2.
>
> For: VIZ-4636
> Signed-off-by: Jeff McGee
> ---
> tests/.gitignore | 1 +
> tests/Makefile.sources | 1 +
> tests/core_getparams.c | 167
> +
> 3 files changed, 169 insertions(+)
> create mode 100644 tests/core_getparams.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 426cc67..c742308 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,6 +1,7 @@
> # Please keep sorted alphabetically
> core_get_client_auth
> core_getclient
> +core_getparams
> core_getstats
> core_getversion
> drm_import_export
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 51e8376..999c8f8 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
>
> TESTS_progs_M = \
> core_get_client_auth \
> + core_getparams \
> drv_suspend \
> drv_hangman \
> gem_bad_reloc \
> diff --git a/tests/core_getparams.c b/tests/core_getparams.c
> new file mode 100644
> index 000..2855d06
> --- /dev/null
> +++ b/tests/core_getparams.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *Jeff McGee
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include "drmtest.h"
> +#include "intel_chipset.h"
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Tests the export of parameters via
> DRM_IOCTL_I915_GETPARAM\n");
> +
> +int drm_fd;
> +int devid;
> +
> +static void
> +init(void)
> +{
> + drm_fd = drm_open_any();
> + devid = intel_get_drm_devid(drm_fd);
> +}
> +
> +static void
> +deinit(void)
> +{
> + close(drm_fd);
> +}
> +
> +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> +#define LOCAL_I915_PARAM_EU_TOTAL34
> +
> +static int
> +getparam(int param, int *value)
> +{
> + drm_i915_getparam_t gp;
> + int ret;
> +
> + memset(&gp, 0, sizeof(gp));
> + gp.value = value;
> + gp.param = param;
> + ret = drmIoctl(drm_fd, DRM_IOCTL_I915_GETPARAM, &gp);
> + if (ret)
> + return -errno;
> +
> + return 0;
> +}
> +
> +static void
> +subslice_total(void)
> +{
> + unsigned int subslice_total = 0;
> + int ret;
> +
> + ret = getparam(LOCAL_I915_PARAM_SUBSLICE_TOTAL, (int*)&subslice_total);
> +
> + if (ret) {
> + /*
> + * These devices are not required to implement the
> + * interface. If they do not, -ENODEV must be returned.
> + */
> + if ((intel_gen(devid)
[Intel-gfx] [patch] drm/i915/skl: cleanup an indenting issue
This statement is indented an extra space. Signed-off-by: Dan Carpenter diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 959058f..028c6d4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3905,13 +3905,13 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) I915_WRITE(GEN6_RP_DOWN_THRESHOLD, GT_INTERVAL_FROM_US(dev_priv, (ei_down * threshold_down / 100))); -I915_WRITE(GEN6_RP_CONTROL, - GEN6_RP_MEDIA_TURBO | - GEN6_RP_MEDIA_HW_NORMAL_MODE | - GEN6_RP_MEDIA_IS_GFX | - GEN6_RP_ENABLE | - GEN6_RP_UP_BUSY_AVG | - GEN6_RP_DOWN_IDLE_AVG); + I915_WRITE(GEN6_RP_CONTROL, + GEN6_RP_MEDIA_TURBO | + GEN6_RP_MEDIA_HW_NORMAL_MODE | + GEN6_RP_MEDIA_IS_GFX | + GEN6_RP_ENABLE | + GEN6_RP_UP_BUSY_AVG | + GEN6_RP_DOWN_IDLE_AVG); dev_priv->rps.power = new_power; dev_priv->rps.last_adj = 0; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/13] drm/i915: Hide the source vs. sink rate handling from intel_dp_compute_config()
On Thursday 12 March 2015 08:40 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
intel_dp_compute_config() only really needs to know the rates supported
by both source and sink, so hide the raw source and sink arrays from it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 37 +++--
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 61538f4..a88f932 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1192,9 +1192,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static int intel_supported_rates(const int *source_rates, int source_len,
-const int *sink_rates, int sink_len,
-int *supported_rates)
+static int intersect_rates(const int *source_rates, int source_len,
+ const int *sink_rates, int sink_len,
+ int *supported_rates)
{
int i = 0, j = 0, k = 0;
@@ -1213,6 +1213,21 @@ static int intel_supported_rates(const int *source_rates, int source_len,
return k;
}
+static int intel_supported_rates(struct intel_dp *intel_dp,
+int *supported_rates)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ const int *source_rates, *sink_rates;
+ int source_len, sink_len;
+
+ sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
+ source_len = intel_dp_source_rates(dev, &source_rates);
+
+ return intersect_rates(source_rates, source_len,
+ sink_rates, sink_len,
+ supported_rates);
+}
+
Now when I am looking at it, the name "supported_rates", sounds
confusing. Because first we are using it for sink_rates in
intel_dp->supported_rates
and then we use it to store the intersect_rates. Since sink_rates are
termed supported_rates in the spec, can we remove the sink_rates altogether
and have source_rates, sink_supported_rates and common_rates (or
something else ?)
-Sonika
static int rate_to_index(int find, const int *rates)
{
int i = 0;
@@ -1243,17 +1258,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
- const int *sink_rates;
- int supported_rates[8] = {0};
- const int *source_rates;
- int source_len, sink_len, supported_len;
-
- sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
-
- source_len = intel_dp_source_rates(dev, &source_rates);
+ int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
+ int supported_len;
- supported_len = intel_supported_rates(source_rates, source_len,
- sink_rates, sink_len, supported_rates);
+ supported_len = intel_supported_rates(intel_dp, supported_rates);
/* No common link rates between source and sink */
WARN_ON(supported_len <= 0);
@@ -1352,7 +1360,8 @@ found:
if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
intel_dp->rate_select =
- rate_to_index(supported_rates[clock], sink_rates);
+ rate_to_index(supported_rates[clock],
+ intel_dp->supported_rates);
intel_dp->link_bw = 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/13] drm/i915: Hide the source vs. sink rate handling from intel_dp_compute_config()
On Fri, Mar 13, 2015 at 05:14:59PM +0530, sonika wrote:
>
> On Thursday 12 March 2015 08:40 PM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > intel_dp_compute_config() only really needs to know the rates supported
> > by both source and sink, so hide the raw source and sink arrays from it.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 37 +++--
> > 1 file changed, 23 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 61538f4..a88f932 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1192,9 +1192,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
> > }
> > }
> >
> > -static int intel_supported_rates(const int *source_rates, int source_len,
> > -const int *sink_rates, int sink_len,
> > -int *supported_rates)
> > +static int intersect_rates(const int *source_rates, int source_len,
> > + const int *sink_rates, int sink_len,
> > + int *supported_rates)
> > {
> > int i = 0, j = 0, k = 0;
> >
> > @@ -1213,6 +1213,21 @@ static int intel_supported_rates(const int
> > *source_rates, int source_len,
> > return k;
> > }
> >
> > +static int intel_supported_rates(struct intel_dp *intel_dp,
> > +int *supported_rates)
> > +{
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + const int *source_rates, *sink_rates;
> > + int source_len, sink_len;
> > +
> > + sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> > + source_len = intel_dp_source_rates(dev, &source_rates);
> > +
> > + return intersect_rates(source_rates, source_len,
> > + sink_rates, sink_len,
> > + supported_rates);
> > +}
> > +
> Now when I am looking at it, the name "supported_rates", sounds
> confusing. Because first we are using it for sink_rates in
> intel_dp->supported_rates
> and then we use it to store the intersect_rates. Since sink_rates are
> termed supported_rates in the spec, can we remove the sink_rates altogether
> and have source_rates, sink_supported_rates and common_rates (or
> something else ?)
Hmm. Yeah. Maybe just rename to intel_dp->sink_rates? And common_rates
would seem like a decent name for the intersection.
>
> -Sonika
> > static int rate_to_index(int find, const int *rates)
> > {
> > int i = 0;
> > @@ -1243,17 +1258,10 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> > int max_clock;
> > int bpp, mode_rate;
> > int link_avail, link_clock;
> > - const int *sink_rates;
> > - int supported_rates[8] = {0};
> > - const int *source_rates;
> > - int source_len, sink_len, supported_len;
> > -
> > - sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> > -
> > - source_len = intel_dp_source_rates(dev, &source_rates);
> > + int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
> > + int supported_len;
> >
> > - supported_len = intel_supported_rates(source_rates, source_len,
> > - sink_rates, sink_len, supported_rates);
> > + supported_len = intel_supported_rates(intel_dp, supported_rates);
> >
> > /* No common link rates between source and sink */
> > WARN_ON(supported_len <= 0);
> > @@ -1352,7 +1360,8 @@ found:
> >
> > if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
> > intel_dp->rate_select =
> > - rate_to_index(supported_rates[clock], sink_rates);
> > + rate_to_index(supported_rates[clock],
> > + intel_dp->supported_rates);
> > intel_dp->link_bw = 0;
> > }
> >
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/13] drm/i915: Hide the source vs. sink rate handling from intel_dp_compute_config()
On Friday 13 March 2015 05:32 PM, Ville Syrjälä wrote:
On Fri, Mar 13, 2015 at 05:14:59PM +0530, sonika wrote:
On Thursday 12 March 2015 08:40 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
intel_dp_compute_config() only really needs to know the rates supported
by both source and sink, so hide the raw source and sink arrays from it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 37 +++--
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 61538f4..a88f932 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1192,9 +1192,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static int intel_supported_rates(const int *source_rates, int source_len,
-const int *sink_rates, int sink_len,
-int *supported_rates)
+static int intersect_rates(const int *source_rates, int source_len,
+ const int *sink_rates, int sink_len,
+ int *supported_rates)
{
int i = 0, j = 0, k = 0;
@@ -1213,6 +1213,21 @@ static int intel_supported_rates(const int *source_rates, int source_len,
return k;
}
+static int intel_supported_rates(struct intel_dp *intel_dp,
+int *supported_rates)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ const int *source_rates, *sink_rates;
+ int source_len, sink_len;
+
+ sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
+ source_len = intel_dp_source_rates(dev, &source_rates);
+
+ return intersect_rates(source_rates, source_len,
+ sink_rates, sink_len,
+ supported_rates);
+}
+
Now when I am looking at it, the name "supported_rates", sounds
confusing. Because first we are using it for sink_rates in
intel_dp->supported_rates
and then we use it to store the intersect_rates. Since sink_rates are
termed supported_rates in the spec, can we remove the sink_rates altogether
and have source_rates, sink_supported_rates and common_rates (or
something else ?)
Hmm. Yeah. Maybe just rename to intel_dp->sink_rates? And common_rates
would seem like a decent name for the intersection.
Sounds good.
-Sonika
static int rate_to_index(int find, const int *rates)
{
int i = 0;
@@ -1243,17 +1258,10 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
- const int *sink_rates;
- int supported_rates[8] = {0};
- const int *source_rates;
- int source_len, sink_len, supported_len;
-
- sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
-
- source_len = intel_dp_source_rates(dev, &source_rates);
+ int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
+ int supported_len;
- supported_len = intel_supported_rates(source_rates, source_len,
- sink_rates, sink_len, supported_rates);
+ supported_len = intel_supported_rates(intel_dp, supported_rates);
/* No common link rates between source and sink */
WARN_ON(supported_len <= 0);
@@ -1352,7 +1360,8 @@ found:
if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
intel_dp->rate_select =
- rate_to_index(supported_rates[clock], sink_rates);
+ rate_to_index(supported_rates[clock],
+ intel_dp->supported_rates);
intel_dp->link_bw = 0;
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Ensure plane->state->fb stays in sync with plane->fb
On Thu, 12 Mar 2015, Matt Roper wrote:
> On Thu, Mar 12, 2015 at 02:45:31PM +0200, Jani Nikula wrote:
>>
>> Matt, please review or suggest an alternative for v4.0-rc.
>>
>> Thanks,
>> Jani.
>
> Yep, this looks good to me. So
>
> Reviewed-by: Matt Roper
Pushed to drm-intel-fixes, thanks for the patch and review.
BR,
Jani.
>
>
> Matt
>
>>
>>
>> On Thu, 12 Mar 2015, Xi Ruoyao wrote:
>> > plane->state->fb and plane->fb should always reference the same FB so
>> > that atomic and legacy codepaths have the same view of display state.
>> > However, there are some places in kernel code that directly set
>> > plane->fb and neglect to update plane->state->fb. If we never do a
>> > successful update through the atomic pipeline, the RmFB cleanup code
>> > will look at the plane->state->fb pointer, which has never actually
>> > been set to a legitimate value, and try to clean it up, leading to
>> > BUG's.
>> >
>> > Add a quick helper function to synchronize plane->state->fb with
>> > plane->fb and call it everywhere the driver tries to manually set
>> > plane->fb outside of the atomic pipeline. In this function, use
>> > drm_atomic_set_fb_for_plane instead of writing plane->state->fb
>> > directly to keep the reference count right.
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88909
>> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=93711
>> > Signed-off-by: Matt Roper
>> > Signed-off-by: Daniel Vetter
>> > Signed-off-by: Xi Ruoyao
>> > ---
>> > This is modified from Matt Roper's patch to drm-intel-nightly with
>> > commit id
>> >
>> >afd65eb4cc0578a9c07d621acdb8a570e2782bf7
>> >
>> > However this bug exists in mainline kernel too, so I created this to
>> > fix it in mainline kernel.
>> >
>> > A minor change is to use drm_atomic_set_fb_for_plane instead of
>> > update reference count manually.
>> >
>> > drivers/gpu/drm/i915/intel_display.c | 17 +
>> > 1 file changed, 17 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index e730789..c483f2f 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -37,6 +37,7 @@
>> > #include
>> > #include "i915_drv.h"
>> > #include "i915_trace.h"
>> > +#include
>> > #include
>> > #include
>> > #include
>> > @@ -2416,6 +2417,14 @@ out_unref_obj:
>> >return false;
>> > }
>> >
>> > +/* Update plane->state->fb to match plane->fb after driver-internal
>> > updates */
>> > +static void
>> > +update_state_fb(struct drm_plane *plane)
>> > +{
>> > + if (plane->fb != plane->state->fb)
>> > + drm_atomic_set_fb_for_plane(plane->state, plane->fb);
>> > +}
>> > +
>> > static void
>> > intel_find_plane_obj(struct intel_crtc *intel_crtc,
>> > struct intel_initial_plane_config *plane_config)
>> > @@ -2462,6 +2471,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
>> >break;
>> >}
>> >}
>> > +
>> > + update_state_fb(intel_crtc->base.primary);
>> > }
>> >
>> > static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>> > @@ -6650,6 +6661,7 @@ i9xx_get_initial_plane_config(struct intel_crtc
>> > *crtc,
>> > plane_config->size);
>> >
>> >crtc->base.primary->fb = fb;
>> > + update_state_fb(crtc->base.primary);
>> > }
>> >
>> > static void chv_crtc_clock_get(struct intel_crtc *crtc,
>> > @@ -7687,6 +7699,7 @@ skylake_get_initial_plane_config(struct intel_crtc
>> > *crtc,
>> > plane_config->size);
>> >
>> >crtc->base.primary->fb = fb;
>> > + update_state_fb(crtc->base.primary);
>> >return;
>> >
>> > error:
>> > @@ -7778,6 +7791,7 @@ ironlake_get_initial_plane_config(struct intel_crtc
>> > *crtc,
>> > plane_config->size);
>> >
>> >crtc->base.primary->fb = fb;
>> > + update_state_fb(crtc->base.primary);
>> > }
>> >
>> > static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>> > @@ -9816,6 +9830,7 @@ static int intel_crtc_page_flip(struct drm_crtc
>> > *crtc,
>> >drm_gem_object_reference(&obj->base);
>> >
>> >crtc->primary->fb = fb;
>> > + update_state_fb(crtc->primary);
>> >
>> >work->pending_flip_obj = obj;
>> >
>> > @@ -9884,6 +9899,7 @@ cleanup_unpin:
>> > cleanup_pending:
>> >atomic_dec(&intel_crtc->unpin_work_count);
>> >crtc->primary->fb = old_fb;
>> > + update_state_fb(crtc->primary);
>> >drm_gem_object_unreference(&work->old_fb_obj->base);
>> >drm_gem_object_unreference(&obj->base);
>> >mutex_unlock(&dev->struct_mutex);
>> > @@ -13718,6 +13734,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
>> > to_intel_crtc(c)->pipe);
>> >drm_framebuffer_unreference(c->primary->fb);
>> >c->primary->fb = NULL;
>> > + update_state_fb(c->primary);
>> >}
>> >}
>> >mutex_unlock(&de
[Intel-gfx] [PATCH] drm/i915: Fix vmap_batch page iterator overrun
vmap_batch() calculates amount of needed pages for the mapping
we are going to create. And it uses this page count as an
argument for the for_each_sg_pages() macro. The macro takes the number
of sg list entities as an argument, not the page count. So we ended
up iterating through all the pages on the mapped object, corrupting
memory past the smaller pages[] array.
Fix this by bailing out when we have enough pages.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9a6da35..61ae8ff 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -836,8 +836,11 @@ static u32 *vmap_batch(struct drm_i915_gem_object *obj,
}
i = 0;
- for_each_sg_page(obj->pages->sgl, &sg_iter, npages, first_page)
+ for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,
first_page) {
pages[i++] = sg_page_iter_page(&sg_iter);
+ if (i == npages)
+ break;
+ }
addr = vmap(pages, i, 0, PAGE_KERNEL);
if (addr == NULL) {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'
On 10/03/2015 10:18, Daniel Vetter wrote:
On Mon, Mar 09, 2015 at 11:51:26PM +, Tomas Elf wrote:
On 19/02/2015 17:18, john.c.harri...@intel.com wrote:
From: John Harrison
The outstanding_lazy_request is no longer used anywhere in the driver.
Everything that was looking at it now has a request explicitly passed in from on
high. Everything that was relying upon behind the scenes is now explicitly
creating/passing/submitting it's own private request. Thus the OLR can be
removed.
"Everything that was relying upon behind the scenes is now explicitly
creating/passing/submitting it's ..."
--->
"Everything that was relying on it behind the scenes is now explicitly
creating/passing/submitting its ..." ?
For: VIZ-5115
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/i915_gem.c| 16 +---
drivers/gpu/drm/i915/i915_gem_execbuffer.c |4 +---
drivers/gpu/drm/i915/intel_lrc.c |6 ++
drivers/gpu/drm/i915/intel_ringbuffer.c| 17 ++---
drivers/gpu/drm/i915/intel_ringbuffer.h|4
5 files changed, 6 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 60f6671..8e7418b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1156,15 +1156,9 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
int
i915_gem_check_olr(struct drm_i915_gem_request *req)
{
- int ret;
-
WARN_ON(!mutex_is_locked(&req->ring->dev->struct_mutex));
- ret = 0;
- if (req == req->ring->outstanding_lazy_request)
- ret = i915_add_request(req);
-
- return ret;
+ return 0;
}
static void fake_irq(unsigned long data)
@@ -2424,8 +2418,6 @@ int __i915_add_request(struct drm_i915_gem_request
*request,
dev_priv = ring->dev->dev_private;
ringbuf = request->ringbuf;
- WARN_ON(request != ring->outstanding_lazy_request);
-
request_start = intel_ring_get_tail(ringbuf);
/*
* Emit any outstanding flushes - execbuf can fail to emit the flush
@@ -2486,7 +2478,6 @@ int __i915_add_request(struct drm_i915_gem_request
*request,
}
trace_i915_gem_request_add(request);
- ring->outstanding_lazy_request = NULL;
i915_queue_hangcheck(ring->dev);
@@ -2672,9 +2663,6 @@ static void i915_gem_reset_ring_cleanup(struct
drm_i915_private *dev_priv,
i915_gem_free_request(request);
}
-
- /* This may not have been flushed before the reset, so clean it now */
- i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
}
void i915_gem_restore_fences(struct drm_device *dev)
@@ -3124,8 +3112,6 @@ int i915_gpu_idle(struct drm_device *dev)
}
}
- WARN_ON(ring->outstanding_lazy_request);
-
ret = intel_ring_idle(ring);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 6a703e6..0eae592 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1571,10 +1571,8 @@ err:
* must be freed again. If it was submitted then it is being tracked
* on the active request list and no clean up is required here.
*/
- if (ret && params->request) {
+ if (ret && params->request)
i915_gem_request_unreference(params->request);
- ring->outstanding_lazy_request = NULL;
- }
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2911cf6..db63ea0 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1032,8 +1032,7 @@ int intel_logical_ring_alloc_request(struct
intel_engine_cs *ring,
if (!req_out)
return -EINVAL;
- if ((*req_out = ring->outstanding_lazy_request) != NULL)
- return 0;
+ *req_out = NULL;
request = kzalloc(sizeof(*request), GFP_KERNEL);
if (request == NULL)
@@ -1067,7 +1066,7 @@ int intel_logical_ring_alloc_request(struct
intel_engine_cs *ring,
i915_gem_context_reference(request->ctx);
request->ringbuf = ctx->engine[ring->id].ringbuf;
- *req_out = ring->outstanding_lazy_request = request;
+ *req_out = request;
return 0;
}
@@ -1393,7 +1392,6 @@ void intel_logical_ring_cleanup(struct intel_engine_cs
*ring)
intel_logical_ring_stop(ring);
WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
- i915_gem_request_assign(&ring->outstanding_lazy_request, NULL);
if (ring->cleanup)
ring->cleanup(ring);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5eef02e..85daa18 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++
Re: [Intel-gfx] [PATCH 20/53] drm/i915: Update ppgtt_init_ring() & context_enable() to take requests
On 13/03/2015 12:46, John Harrison wrote:
On 05/03/2015 17:57, Tomas Elf wrote:
On 19/02/2015 17:17, john.c.harri...@intel.com wrote:
From: John Harrison
The final step in removing the OLR from i915_gem_init_hw() is to pass
the newly
allocated request structure in to each step rather than passing a ring
structure. This patch updates both i915_ppgtt_init_ring() and
i915_gem_context_enable() to take request pointers.
For: VIZ-5115
Signed-off-by: John Harrison
---
drivers/gpu/drm/i915/i915_drv.h |2 +-
drivers/gpu/drm/i915/i915_gem.c |4 ++--
drivers/gpu/drm/i915/i915_gem_context.c |7 ---
drivers/gpu/drm/i915/i915_gem_gtt.c |6 +++---
drivers/gpu/drm/i915/i915_gem_gtt.h |2 +-
5 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ea0da6b..618a841 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2990,7 +2990,7 @@ int __must_check i915_gem_context_init(struct
drm_device *dev);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file
*file);
-int i915_gem_context_enable(struct intel_engine_cs *ring);
+int i915_gem_context_enable(struct drm_i915_gem_request *req);
void i915_gem_context_close(struct drm_device *dev, struct drm_file
*file);
int i915_switch_context(struct intel_engine_cs *ring,
struct intel_context *to);
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index efed49a..379bf44 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4877,7 +4877,7 @@ i915_gem_init_hw(struct drm_device *dev)
i915_gem_l3_remap(ring, i);
}
-ret = i915_ppgtt_init_ring(ring);
+ret = i915_ppgtt_init_ring(req);
if (ret && ret != -EIO) {
DRM_ERROR("PPGTT enable ring #%d failed %d\n", i, ret);
i915_gem_request_unreference(req);
@@ -4885,7 +4885,7 @@ i915_gem_init_hw(struct drm_device *dev)
return ret;
}
-ret = i915_gem_context_enable(ring);
+ret = i915_gem_context_enable(req);
if (ret && ret != -EIO) {
DRM_ERROR("Context enable ring #%d failed %d\n", i, ret);
i915_gem_request_unreference(req);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index dd83d61..04d2a20 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -403,17 +403,18 @@ void i915_gem_context_fini(struct drm_device *dev)
i915_gem_context_unreference(dctx);
}
-int i915_gem_context_enable(struct intel_engine_cs *ring)
+int i915_gem_context_enable(struct drm_i915_gem_request *req)
{
+struct intel_engine_cs *ring = req->ring;
int ret;
if (i915.enable_execlists) {
if (ring->init_context == NULL)
return 0;
-ret = ring->init_context(ring, ring->default_context);
+ret = ring->init_context(req->ring, ring->default_context);
} else
-ret = i915_switch_context(ring, ring->default_context);
+ret = i915_switch_context(req->ring, ring->default_context);
You don't have to make any more changes to this function aside from
setting up the ring variable at the top. ring = req->ring and if you
don't change these lines the will by default use ring like they always
did.
if (ret) {
DRM_ERROR("ring init context: %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 428d2f6..cd00080 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1227,15 +1227,15 @@ int i915_ppgtt_init_hw(struct drm_device *dev)
return 0;
}
-int i915_ppgtt_init_ring(struct intel_engine_cs *ring)
+int i915_ppgtt_init_ring(struct drm_i915_gem_request *req)
{
-struct drm_i915_private *dev_priv = ring->dev->dev_private;
+struct drm_i915_private *dev_priv = req->ring->dev->dev_private;
struct i915_hw_ppgtt *ppgtt = dev_priv->mm.aliasing_ppgtt;
if (!ppgtt)
return 0;
-return ppgtt->switch_mm(ppgtt, ring);
+return ppgtt->switch_mm(ppgtt, req->ring);
}
If you want to uphold a pattern that you've already established you
could just make a single change in the function above by setting up
ring = req->ring and then make no more changes to the function body.
In this case it's one new line vs. two changes of existing code so
it's doesn't make that much of a difference but it is nice to stick to
patterns. Also, you wouldn't have to make a 4-level indirection
(req->ring->dev->dev_private), only a 3-level one.
It all depends how often the ring variable would get used. In this case,
there are only two references. One of which, the switch_mm(), will
d
[Intel-gfx] [PATCH 4/9] drivers/pwm: Add helper to configure pwm using clock divisor and duty percent
Some chips instead of using period_ns and duty_ns can be configured
using the clock divisor and duty percent. Adds an alternative
configuration method for such chips
v2: Corrected the chip validation for config_alternate in pwmchip_add
CC: Samuel Ortiz
Cc: Linus Walleij
Cc: Alexandre Courbot
Cc: Thierry Reding
Signed-off-by: Shobhit Kumar
---
drivers/pwm/core.c | 27 ++-
include/linux/pwm.h | 33 +
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 810aef3..604e93d 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -235,7 +235,8 @@ int pwmchip_add(struct pwm_chip *chip)
unsigned int i;
int ret;
- if (!chip || !chip->dev || !chip->ops || !chip->ops->config ||
+ if (!chip || !chip->dev || !chip->ops ||
+ !(chip->ops->config || chip->ops->config_alternate) ||
!chip->ops->enable || !chip->ops->disable || !chip->npwm)
return -EINVAL;
@@ -422,6 +423,30 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int
period_ns)
EXPORT_SYMBOL_GPL(pwm_config);
/**
+ * pwm_config_alternate() - change a PWM device configuration
+ * @pwm: PWM device
+ * @clk_div: Clock divider to configure
+ * @duty_percentage: duty cycle in percentage
+ */
+int pwm_config_alternate(struct pwm_device *pwm, int clk_div, int duty_percent)
+{
+ int err;
+
+ if (!pwm || clk_div < 0 || duty_percent < 0 || duty_percent > 100)
+ return -EINVAL;
+
+ err = pwm->chip->ops->config_alternate(pwm->chip, pwm, clk_div,
duty_percent);
+ if (err)
+ return err;
+
+ pwm->clk_div = clk_div;
+ pwm->duty_percent = duty_percent;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pwm_config_alternate);
+
+/**
* pwm_set_polarity() - configure the polarity of a PWM signal
* @pwm: PWM device
* @polarity: new polarity of the PWM signal
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index e90628c..739cb2b 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -24,6 +24,12 @@ void pwm_free(struct pwm_device *pwm);
int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);
/*
+ * pwm_config_alternate - change a PWM device configuration
+ * based on clk_dic and duty_percent
+ */
+int pwm_config_alternate(struct pwm_device *pwm, int clk_div, int
duty_percent);
+
+/*
* pwm_enable - start a PWM output toggling
*/
int pwm_enable(struct pwm_device *pwm);
@@ -89,6 +95,8 @@ struct pwm_device {
unsigned intperiod; /* in nanoseconds */
unsigned intduty_cycle; /* in nanoseconds */
+ unsigned intclk_div;
+ unsigned intduty_percent;
enum pwm_polarity polarity;
};
@@ -114,6 +122,28 @@ static inline unsigned int pwm_get_duty_cycle(struct
pwm_device *pwm)
return pwm ? pwm->duty_cycle : 0;
}
+static inline void pwm_set_clk_div(struct pwm_device *pwm, unsigned int
clk_div)
+{
+ if (pwm)
+ pwm->clk_div = clk_div;
+}
+
+static inline unsigned int pwm_get_clk_div(struct pwm_device *pwm)
+{
+ return pwm ? pwm->clk_div : 0;
+}
+
+static inline void pwm_set_duty_percent(struct pwm_device *pwm, unsigned int
duty_percent)
+{
+ if (pwm)
+ pwm->duty_percent = duty_percent;
+}
+
+static inline unsigned int pwm_get_duty_percent(struct pwm_device *pwm)
+{
+ return pwm ? pwm->duty_percent : 0;
+}
+
/*
* pwm_set_polarity - configure the polarity of a PWM signal
*/
@@ -138,6 +168,9 @@ struct pwm_ops {
int (*config)(struct pwm_chip *chip,
struct pwm_device *pwm,
int duty_ns, int period_ns);
+ int (*config_alternate)(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ int clk_div, int duty_percent);
int (*set_polarity)(struct pwm_chip *chip,
struct pwm_device *pwm,
enum pwm_polarity polarity);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix vmap_batch page iterator overrun
On 03/13/2015 01:21 PM, Mika Kuoppala wrote:
vmap_batch() calculates amount of needed pages for the mapping
we are going to create. And it uses this page count as an
argument for the for_each_sg_pages() macro. The macro takes the number
of sg list entities as an argument, not the page count. So we ended
up iterating through all the pages on the mapped object, corrupting
memory past the smaller pages[] array.
Fix this by bailing out when we have enough pages.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9a6da35..61ae8ff 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -836,8 +836,11 @@ static u32 *vmap_batch(struct drm_i915_gem_object *obj,
}
i = 0;
- for_each_sg_page(obj->pages->sgl, &sg_iter, npages, first_page)
+ for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents,
first_page) {
pages[i++] = sg_page_iter_page(&sg_iter);
+ if (i == npages)
+ break;
+ }
Are you sure this manual check is needed now that you fixed
for_each_sg_page?
Pages array looks pessimistically big enough so I don't see that memory
was getting overwritten. It looks more like our sg table was not
properly terminated which made for_each_sg_page go into random memory
and return random page pointers.
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix vmap_batch page iterator overrun
On Fri, Mar 13, 2015 at 03:21:53PM +0200, Mika Kuoppala wrote: > vmap_batch() calculates amount of needed pages for the mapping > we are going to create. And it uses this page count as an > argument for the for_each_sg_pages() macro. The macro takes the number > of sg list entities as an argument, not the page count. So we ended > up iterating through all the pages on the mapped object, corrupting > memory past the smaller pages[] array. > > Fix this by bailing out when we have enough pages. > > Cc: Chris Wilson > Signed-off-by: Mika Kuoppala Reviewed-by: Chris Wilson Can I ask for a st_for_each_page(&obj->pages, &sg_iter, n)? That would simplify all of our users, and stop me from making the same mistake again. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests: create a single combined test list
All tests now respond in a consistent way such that separate lists for
tests with and without subtests are no longer necessary.
Signed-off-by: Thomas Wood
---
tests/Makefile.am | 23 ---
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5efc8d8..d808973 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -5,32 +5,17 @@ if HAVE_NOUVEAU
endif
if BUILD_TESTS
-all-local: single-tests.txt multi-tests.txt
+all-local: test-list.txt
-list-single-tests:
- @echo TESTLIST
- @echo ${single_kernel_tests}
- @echo END TESTLIST
-
-list-multi-tests:
- @echo TESTLIST
- @echo ${multi_kernel_tests}
- @echo END TESTLIST
-
-single-tests.txt: Makefile.sources
- @echo TESTLIST > $@
- @echo ${single_kernel_tests} >> $@
- @echo END TESTLIST >> $@
-
-multi-tests.txt: Makefile.sources
+test-list.txt: Makefile.sources
@echo TESTLIST > $@
- @echo ${multi_kernel_tests} >> $@
+ @echo ${single_kernel_tests} ${multi_kernel_tests} >> $@
@echo END TESTLIST >> $@
EXTRA_PROGRAMS = $(TESTS_progs) $(TESTS_progs_M) $(HANG)
EXTRA_DIST = $(TESTS_scripts) $(TESTS_scripts_M) $(scripts) $(IMAGES)
$(common_files)
-CLEANFILES = $(EXTRA_PROGRAMS) single-tests.txt multi-tests.txt
+CLEANFILES = $(EXTRA_PROGRAMS) test-list.txt
AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
-I$(srcdir)/.. \
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v5 3/9] drm/i915: Use the CRC gpio for panel enable/disable
On Thu, Mar 12, 2015 at 10:01:27PM +0530, Shobhit Kumar wrote:
> The CRC (Crystal Cove) PMIC, controls the panel enable and disable
> signals for BYT for dsi panels. This is indicated in the VBT fields. Use
> that to initialize and use GPIO based control for these signals.
>
> v2: Use the newer gpiod interface(Alexandre)
I definitely like how this looks. No pmic specific information has
leaked into i915. I can't really comment on the gpio/pwm specific bits
as I'm all that familiar with them, but based on a cursory look it
seemed pretty nice as well.
Just a few small bikesheds below about the i915 bits.
>
> CC: Samuel Ortiz
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Thierry Reding
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 34 --
> drivers/gpu/drm/i915/intel_dsi.h | 10 ++
> 2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index c8c8b24..219421c 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include "i915_drv.h"
> #include "intel_drv.h"
> #include "intel_dsi.h"
> @@ -415,6 +416,13 @@ static void intel_dsi_pre_enable(struct intel_encoder
> *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> + /* Panel Enable over CRC PMIC */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC &&
> + intel_dsi->gpio_panel)
The vbt check here seems redundant.
> + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> +
> + msleep(intel_dsi->panel_on_delay);
> +
> /* Disable DPOunit clock gating, can stall pipe
>* and we need DPLL REFA always enabled */
> tmp = I915_READ(DPLL(pipe));
> @@ -432,8 +440,6 @@ static void intel_dsi_pre_enable(struct intel_encoder
> *encoder)
> /* put device in ready state */
> intel_dsi_device_ready(encoder);
>
> - msleep(intel_dsi->panel_on_delay);
> -
> drm_panel_prepare(intel_dsi->panel);
>
> for_each_dsi_port(port, intel_dsi->ports)
> @@ -576,6 +582,11 @@ static void intel_dsi_post_disable(struct intel_encoder
> *encoder)
>
> msleep(intel_dsi->panel_off_delay);
> msleep(intel_dsi->panel_pwr_cycle_delay);
> +
> + /* Panel Disable over CRC PMIC */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC &&
> + intel_dsi->gpio_panel)
ditto
> + gpiod_set_value_cansleep(intel_dsi->gpio_panel, 0);
> }
>
> static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> @@ -955,6 +966,11 @@ static void intel_dsi_encoder_destroy(struct drm_encoder
> *encoder)
> /* XXX: Logically this call belongs in the panel driver. */
> drm_panel_remove(intel_dsi->panel);
> }
> +
> + /* dispose of the gpios */
> + if (intel_dsi->gpio_panel)
> + gpiod_put(intel_dsi->gpio_panel);
> +
> intel_encoder_destroy(encoder);
> }
>
> @@ -1070,6 +1086,20 @@ void intel_dsi_init(struct drm_device *dev)
> goto err;
> }
>
> + /*
> + * In case of BYT with CRC PMIC, we need to use GPIO for
> + * Panel control.
> + */
> + if (dev_priv->vbt.dsi.config->pwm_blc == PPS_BLC_PMIC) {
> + intel_dsi->gpio_panel =
> + gpiod_get(dev->dev, "panel", GPIOD_OUT_HIGH);
> +
> + if (IS_ERR(intel_dsi->gpio_panel)) {
> + DRM_ERROR("Failed to own gpio for panel control\n");
> + intel_dsi->gpio_panel = NULL;
> + }
> + }
> +
> intel_encoder->type = INTEL_OUTPUT_DSI;
> intel_encoder->cloneable = 0;
> drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 2784ac4..8be75a7 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -29,6 +29,13 @@
> #include
> #include "intel_drv.h"
>
> +/* CRC PMIC GPIO Access */
> +#define GPIO_CHIP_NAME "gpio_crystalcove"
> +#define GPIO_PANEL_EN94
These defines seem to be unused.
> +
> +#define PPS_BLC_PMIC 0
> +#define PPS_BLC_SOC1
> +
> /* Dual Link support */
> #define DSI_DUAL_LINK_NONE 0
> #define DSI_DUAL_LINK_FRONT_BACK 1
> @@ -42,6 +49,9 @@ struct intel_dsi {
> struct drm_panel *panel;
> struct intel_dsi_host *dsi_hosts[I915_MAX_PORTS];
>
> + /* GPIO Desc for CRC based Panel control */
> + struct gpio_desc *gpio_panel;
> +
> struct intel_connector *attached_connector;
>
> /* bit mask of ports being driven */
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.
Re: [Intel-gfx] [RFC v5 9/9] drm/i915: Backlight control using CRC PMIC based PWM driver
On Thu, Mar 12, 2015 at 10:01:33PM +0530, Shobhit Kumar wrote:
> CC: Samuel Ortiz
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Thierry Reding
> Signed-off-by: Shobhit Kumar
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 25 +
> drivers/gpu/drm/i915/intel_dsi.h | 3 +++
> 2 files changed, 28 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 219421c..511446f 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -32,6 +32,7 @@
> #include
> #include
> #include
> +#include
> #include "i915_drv.h"
> #include "intel_drv.h"
> #include "intel_dsi.h"
> @@ -402,6 +403,10 @@ static void intel_dsi_enable(struct intel_encoder
> *encoder)
>
> intel_dsi_port_enable(encoder);
> }
> +
> + /* Enable the backlight with default PWM as programmed by BIOS */
> + pwm_enable(intel_dsi->pwm);
> + pwm_config_alternate(intel_dsi->pwm, 0x7F, 100);
I suppose we migth want to wrap these in 'if (intel_dsi->pwm)' checks,
or does the pwm subsystem take care of NULL checks for us?
> }
>
> static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> @@ -467,6 +472,11 @@ static void intel_dsi_pre_disable(struct intel_encoder
> *encoder)
>
> DRM_DEBUG_KMS("\n");
>
> + /* Disable the backlight */
> + pwm_config_alternate(intel_dsi->pwm, 0x7F, 0);
> + usleep_range(2000, 3000);
> + pwm_disable(intel_dsi->pwm);
ditto
> +
> if (is_vid_mode(intel_dsi)) {
> /* Send Shutdown command to the panel in LP mode */
> for_each_dsi_port(port, intel_dsi->ports)
> @@ -971,6 +981,10 @@ static void intel_dsi_encoder_destroy(struct drm_encoder
> *encoder)
> if (intel_dsi->gpio_panel)
> gpiod_put(intel_dsi->gpio_panel);
>
> + /* dispose of the pwm */
> + if (intel_dsi->pwm)
> + pwm_put(intel_dsi->pwm);
> +
> intel_encoder_destroy(encoder);
> }
>
> @@ -1098,6 +1112,17 @@ void intel_dsi_init(struct drm_device *dev)
> DRM_ERROR("Failed to own gpio for panel control\n");
> intel_dsi->gpio_panel = NULL;
> }
> +
> + /* Get the PWM chip for backlight control */
> + intel_dsi->pwm = pwm_get(dev->dev, "pwm_backlight");
> + if (IS_ERR(intel_dsi->pwm)) {
> + DRM_ERROR("Faild to own the pwm chip\n");
> + intel_dsi->pwm = NULL;
> + } else if (pwm_config_alternate(intel_dsi->pwm, 0x7F, 100) < 0)
> {
> + DRM_ERROR("Failed to configure the pwm chip\n");
> + pwm_put(intel_dsi->pwm);
> + intel_dsi->pwm = NULL;
> + }
> }
>
> intel_encoder->type = INTEL_OUTPUT_DSI;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index 8be75a7..d8ec9c1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -52,6 +52,9 @@ struct intel_dsi {
> /* GPIO Desc for CRC based Panel control */
> struct gpio_desc *gpio_panel;
>
> + /* PWM chip */
> + struct pwm_device *pwm;
> +
> struct intel_connector *attached_connector;
>
> /* bit mask of ports being driven */
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/5] drm/dp: i2c-over-aux short write support
From: Ville Syrjälä This series tries to implement support for short i2c-over-aux writes. I did notice that my monitor (HP ZR24w) does support DDC/CI so I was able to do some writes, but I only saw native defers instead of i2c defers/short acks. So I've not actually been able to test this. Another complication with DDC/CI seems to be that the monitor doesn't seem to like the default 100kHz bus speed. I had to drop it to 10kHz to make it reliable (my dongle supports bus speed control fortunately). Currently we have no standard way to configure the bus speed AFAICS, so at least with this monitor DDC/CI is a bit useless. I tried to fix up radeon and tegra too. gma500 I left alone since it's not using dp i2c helper. Ville Syrjälä (5): drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/ drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE drm/radeon: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests drivers/gpu/drm/drm_dp_helper.c | 42 drivers/gpu/drm/i915/intel_dp.c | 1 + drivers/gpu/drm/radeon/atombios_dp.c | 1 + drivers/gpu/drm/tegra/dpaux.c| 3 ++- include/drm/drm_dp_helper.h | 2 +- 5 files changed, 43 insertions(+), 6 deletions(-) -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/radeon: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE
From: Ville Syrjälä
When we get an i2c defer or short ack for i2c-over-aux write we need
to switch to WRITE_STATUS_UPDATE to poll for the completion of the
original request.
Looks like radeon doesn't do anything special with the request type,
so hopefully just treating it the same as a i2c write is enough.
Cc: Alex Deucher
Cc: "Christian König"
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/radeon/atombios_dp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
b/drivers/gpu/drm/radeon/atombios_dp.c
index 8d74de8..9960b69 100644
--- a/drivers/gpu/drm/radeon/atombios_dp.c
+++ b/drivers/gpu/drm/radeon/atombios_dp.c
@@ -178,6 +178,7 @@ radeon_dp_aux_transfer(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
switch (msg->request & ~DP_AUX_I2C_MOT) {
case DP_AUX_NATIVE_WRITE:
case DP_AUX_I2C_WRITE:
+ case DP_AUX_I2C_WRITE_STATUS_UPDATE:
/* The atom implementation only supports writes with a max
payload of
* 12 bytes since it uses 4 bits for the total count (header +
payload)
* in the parameter space. The atom interface supports 16 byte
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/
From: Ville Syrjälä Rename the I2C_STATUS request to I2C_WRITE_STATUS_UPDATE to match the spec. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/tegra/dpaux.c | 2 +- include/drm/drm_dp_helper.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index d6b55e3..b1617af 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -152,7 +152,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, break; - case DP_AUX_I2C_STATUS: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: if (msg->request & DP_AUX_I2C_MOT) value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ; else diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 523f04c..27301f5 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -46,7 +46,7 @@ #define DP_AUX_I2C_WRITE 0x0 #define DP_AUX_I2C_READ0x1 -#define DP_AUX_I2C_STATUS 0x2 +#define DP_AUX_I2C_WRITE_STATUS_UPDATE 0x2 #define DP_AUX_I2C_MOT 0x4 #define DP_AUX_NATIVE_WRITE0x8 #define DP_AUX_NATIVE_READ 0x9 -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/tegra: Handle I2C_WRITE_STATUS_UPDATE for address only writes
From: Ville Syrjälä
A address-only I2C_WRITE can't be replied with a short i2c ack, but I
suppose it could be replied with an i2c defer. So the code should be
prepared for an address-only I2C_WRITE_STATUS_UPDATE.
Cc: Thierry Reding
Cc: "Terje Bergström"
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/tegra/dpaux.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index b1617af..a48687a 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -122,6 +122,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux,
*/
if (msg->size < 1) {
switch (msg->request & ~DP_AUX_I2C_MOT) {
+ case DP_AUX_I2C_WRITE_STATUS_UPDATE:
case DP_AUX_I2C_WRITE:
case DP_AUX_I2C_READ:
value = DPAUX_DP_AUXCTL_CMD_ADDRESS_ONLY;
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE
From: Ville Syrjälä
When we get an i2c defer or short ack for i2c-over-aux write we need
to switch to WRITE_STATUS_UPDATE to poll for the completion of the
original request.
i915 doesn't try to interpret wht request type apart from separating
reads from writes, and so we should be able to treat this the same as
a normal i2c write.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33d5877..aed5f26 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -956,6 +956,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
switch (msg->request & ~DP_AUX_I2C_MOT) {
case DP_AUX_NATIVE_WRITE:
case DP_AUX_I2C_WRITE:
+ case DP_AUX_I2C_WRITE_STATUS_UPDATE:
txsize = msg->size ? HEADER_SIZE + msg->size :
BARE_ADDRESS_SIZE;
rxsize = 1;
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests
From: Ville Syrjälä
When an i2c WRITE gets an i2c defer or short i2c ack reply, we are
supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE
when we continue to poll for the completion of the request.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_dp_helper.c | 41 +
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index d5368ea..4db81a6 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
*adapter)
I2C_FUNC_10BIT_ADDR;
}
+static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg,
+ const struct i2c_msg *i2c_msg)
+{
+ msg->request = (i2c_msg->flags & I2C_M_RD) ?
+ DP_AUX_I2C_READ : DP_AUX_I2C_WRITE;
+ msg->request |= DP_AUX_I2C_MOT;
+}
+
+static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg)
+{
+ /*
+* In case of i2c defer or short i2c ack reply to a write,
+* we need to switch to WRITE_STATUS_UPDATE to drain the
+* rest of the message
+*/
+ if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE)
+ msg->request |= DP_AUX_I2C_WRITE_STATUS_UPDATE;
+}
+
/*
* Transfer a single I2C-over-AUX message and handle various error conditions,
* retrying the transaction as appropriate. It is assumed that the
@@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
* Both native ACK and I2C ACK replies received. We
* can assume the transfer was successful.
*/
+ if (ret != msg->size)
+ drm_dp_i2c_msg_write_status_update(msg);
return ret;
case DP_AUX_I2C_REPLY_NACK:
@@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct
drm_dp_aux_msg *msg)
DRM_DEBUG_KMS("I2C defer\n");
aux->i2c_defer_count++;
usleep_range(400, 500);
+ drm_dp_i2c_msg_write_status_update(msg);
continue;
default:
@@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter,
struct i2c_msg *msgs,
for (i = 0; i < num; i++) {
msg.address = msgs[i].addr;
- msg.request = (msgs[i].flags & I2C_M_RD) ?
- DP_AUX_I2C_READ :
- DP_AUX_I2C_WRITE;
- msg.request |= DP_AUX_I2C_MOT;
+ drm_dp_i2c_msg_set_request(&msg, &msgs[i]);
/* Send a bare address packet to start the transaction.
* Zero sized messages specify an address only (bare
* address) transaction.
@@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter,
struct i2c_msg *msgs,
msg.buffer = NULL;
msg.size = 0;
err = drm_dp_i2c_do_msg(aux, &msg);
+
+ /*
+* Reset msg.request in case in case it got
+* changed into a WRITE_STATUS_UPDATE.
+*/
+ drm_dp_i2c_msg_set_request(&msg, &msgs[i]);
+
if (err < 0)
break;
/* We want each transaction to be as large as possible, but
@@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter,
struct i2c_msg *msgs,
msg.size = min(transfer_size, msgs[i].len - j);
err = drm_dp_i2c_drain_msg(aux, &msg);
+
+ /*
+* Reset msg.request in case in case it got
+* changed into a WRITE_STATUS_UPDATE.
+*/
+ drm_dp_i2c_msg_set_request(&msg, &msgs[i]);
+
if (err < 0)
break;
transfer_size = err;
--
2.0.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote:
> My only concern is about the following macros:
>
> > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> > +#define LOCAL_I915_PARAM_EU_TOTAL34
>
> How about to just use the definitons in the kernel header file?
> For an example:
>
> #include
>
> #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
> //Put all the code into this block.
> #endif
>
> Then we can avoid put the same definitions in different files,
> and we can avoid unecessary testing on an old kernel which doesn't
> have this kernel interface.
>
> For all the other part, it LGTM.
>
> Reviewed-by: Zhigang Gong
Once we update the libdrm requirements in igt we tend to go around and
replace all the now obsolete LOCAL_ defines. Imo not worth doing extra
work until then.
Patch applied, thanks.
-Daniel
>
> Thanks,
> Zhigang Gong.
>
> On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mc...@intel.com wrote:
> > From: Jeff McGee
> >
> > New test core_getparams consists of 2 subtests, each one testing
> > the ability of userspace to query the correct value of a GT config
> > attribute: subslice total or EU total. drm/i915 implementation of
> > these queries is required for Cherryview and Gen9+ devices (non-
> > simulated).
> >
> > v2: Duplicate small amount of new libdrm functionality to avoid
> > bumping libdrm version requirement (Daniel). Convert some
> > igt_asserts to the appropriate comparison variants. Add a
> > test description.
> > v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
> > against older libdrm as intended by v2.
> >
> > For: VIZ-4636
> > Signed-off-by: Jeff McGee
> > ---
> > tests/.gitignore | 1 +
> > tests/Makefile.sources | 1 +
> > tests/core_getparams.c | 167
> > +
> > 3 files changed, 169 insertions(+)
> > create mode 100644 tests/core_getparams.c
> >
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > index 426cc67..c742308 100644
> > --- a/tests/.gitignore
> > +++ b/tests/.gitignore
> > @@ -1,6 +1,7 @@
> > # Please keep sorted alphabetically
> > core_get_client_auth
> > core_getclient
> > +core_getparams
> > core_getstats
> > core_getversion
> > drm_import_export
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 51e8376..999c8f8 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
> >
> > TESTS_progs_M = \
> > core_get_client_auth \
> > + core_getparams \
> > drv_suspend \
> > drv_hangman \
> > gem_bad_reloc \
> > diff --git a/tests/core_getparams.c b/tests/core_getparams.c
> > new file mode 100644
> > index 000..2855d06
> > --- /dev/null
> > +++ b/tests/core_getparams.c
> > @@ -0,0 +1,167 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > next
> > + * paragraph) shall be included in all copies or substantial portions of
> > the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + * Authors:
> > + *Jeff McGee
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "drmtest.h"
> > +#include "intel_chipset.h"
> > +#include "intel_bufmgr.h"
> > +
> > +IGT_TEST_DESCRIPTION("Tests the export of parameters via
> > DRM_IOCTL_I915_GETPARAM\n");
> > +
> > +int drm_fd;
> > +int devid;
> > +
> > +static void
> > +init(void)
> > +{
> > + drm_fd = drm_open_any();
> > + devid = intel_get_drm_devid(drm_fd);
> > +}
> > +
> > +static void
> > +deinit(void)
> > +{
> > + close(drm_fd);
> > +}
> > +
> > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL33
> > +#define LOCAL_I915_PARAM_EU_TOTAL 34
> > +
> > +static int
> > +getparam(int param, int *value)
> > +{
> > + drm_i915_getparam_t gp;
> > + int ret;
> > +
> > + memset(&gp, 0, sizeof(gp));
> > + gp.value = value;
> > + gp.param = param;
> > + ret =
Re: [Intel-gfx] [Beignet] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote:
> On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote:
> > My only concern is about the following macros:
> >
> > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33
> > > +#define LOCAL_I915_PARAM_EU_TOTAL34
> >
> > How about to just use the definitons in the kernel header file?
> > For an example:
> >
> > #include
> >
> > #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL
> > //Put all the code into this block.
> > #endif
> >
> > Then we can avoid put the same definitions in different files,
> > and we can avoid unecessary testing on an old kernel which doesn't
> > have this kernel interface.
> >
> > For all the other part, it LGTM.
> >
> > Reviewed-by: Zhigang Gong
>
> Once we update the libdrm requirements in igt we tend to go around and
> replace all the now obsolete LOCAL_ defines. Imo not worth doing extra
> work until then.
>
> Patch applied, thanks.
> -Daniel
>
Patch applied? Do you want me to make the name change first? Should the
kernel part be reviewed and merged first?
-Jeff
> >
> > Thanks,
> > Zhigang Gong.
> >
> > On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mc...@intel.com wrote:
> > > From: Jeff McGee
> > >
> > > New test core_getparams consists of 2 subtests, each one testing
> > > the ability of userspace to query the correct value of a GT config
> > > attribute: subslice total or EU total. drm/i915 implementation of
> > > these queries is required for Cherryview and Gen9+ devices (non-
> > > simulated).
> > >
> > > v2: Duplicate small amount of new libdrm functionality to avoid
> > > bumping libdrm version requirement (Daniel). Convert some
> > > igt_asserts to the appropriate comparison variants. Add a
> > > test description.
> > > v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build
> > > against older libdrm as intended by v2.
> > >
> > > For: VIZ-4636
> > > Signed-off-by: Jeff McGee
> > > ---
> > > tests/.gitignore | 1 +
> > > tests/Makefile.sources | 1 +
> > > tests/core_getparams.c | 167
> > > +
> > > 3 files changed, 169 insertions(+)
> > > create mode 100644 tests/core_getparams.c
> > >
> > > diff --git a/tests/.gitignore b/tests/.gitignore
> > > index 426cc67..c742308 100644
> > > --- a/tests/.gitignore
> > > +++ b/tests/.gitignore
> > > @@ -1,6 +1,7 @@
> > > # Please keep sorted alphabetically
> > > core_get_client_auth
> > > core_getclient
> > > +core_getparams
> > > core_getstats
> > > core_getversion
> > > drm_import_export
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index 51e8376..999c8f8 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \
> > >
> > > TESTS_progs_M = \
> > > core_get_client_auth \
> > > + core_getparams \
> > > drv_suspend \
> > > drv_hangman \
> > > gem_bad_reloc \
> > > diff --git a/tests/core_getparams.c b/tests/core_getparams.c
> > > new file mode 100644
> > > index 000..2855d06
> > > --- /dev/null
> > > +++ b/tests/core_getparams.c
> > > @@ -0,0 +1,167 @@
> > > +/*
> > > + * Copyright © 2014 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining
> > > a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the
> > > next
> > > + * paragraph) shall be included in all copies or substantial portions of
> > > the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + * Authors:
> > > + *Jeff McGee
> > > + *
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include "drmtest.h"
> > > +#include "intel_chipset.h"
> > > +#include "intel_bufmgr.h"
> > > +
> > > +IGT_TEST_DESCRIPTION("Tests the export of parameters via
> > > DRM_IOCTL_I915_GETPARAM\n");
> > > +
> > > +int drm_fd;
> > > +int devid;
> > > +
> > > +static void
> > > +init(void)
> >
Re: [Intel-gfx] [Beignet] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 09:51:57AM -0700, Jeff McGee wrote: > On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote: > > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote: > > > My only concern is about the following macros: > > > > > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33 > > > > +#define LOCAL_I915_PARAM_EU_TOTAL34 > > > > > > How about to just use the definitons in the kernel header file? > > > For an example: > > > > > > #include > > > > > > #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL > > > //Put all the code into this block. > > > #endif > > > > > > Then we can avoid put the same definitions in different files, > > > and we can avoid unecessary testing on an old kernel which doesn't > > > have this kernel interface. > > > > > > For all the other part, it LGTM. > > > > > > Reviewed-by: Zhigang Gong > > > > Once we update the libdrm requirements in igt we tend to go around and > > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra > > work until then. > > > > Patch applied, thanks. > > -Daniel > > > > Patch applied? Do you want me to make the name change first? Should the > kernel part be reviewed and merged first? Forgot my own review comment already ;-) Fixed up with a follow-up patch. And I'll pull the kernel part in now too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 10:27:38AM +0100, Daniel Vetter wrote: > If supporting systems without full ppgtt is a requirement for you (still > wonky on gen8 a bit, so might be a good strategy) then imo it's the > PIN_BIAS idea I've laid out earlier in this thread. That one will work > everywhere. softpin can unexpectedly fail without full ppgtt if the kernel > decides to put something at a given spot, which imo means we should only > expose it on full ppgtt systems. > > And PIN_BIAS should be fairly easy to wire up since the internal logic is > all there already. So "just" needs an execbuf flag, igt test and > appropriate userspace to set that new bit. It doesn't though. To provide the guarantee userspace is asking for (which is that address 0 goes to a special, preferrably inaccessible, page), you have to evict the first N pages in the GGTT. That is just as likely to fail with an execbuffer flag as it would with an execobject flag. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
On Mon, Mar 09, 2015 at 08:10:06AM +0800, Zhigang Gong wrote: > > -Original Message- > > From: Beignet [mailto:beignet-boun...@lists.freedesktop.org] On Behalf Of > > Jeff McGee > > Sent: Saturday, March 7, 2015 2:44 AM > > To: Zhigang Gong > > Cc: dan...@ffwll.ch; intel-gfx@lists.freedesktop.org; > > beig...@lists.freedesktop.org; dri-de...@lists.freedesktop.org > > Subject: Re: [Beignet] [PATCH] drm/i915: Export total subslice and EU counts > > > > On Thu, Mar 05, 2015 at 12:35:55PM +0800, Zhigang Gong wrote: > > > There is one minor conflict when apply the KMD patch to latest > > > drm-intel-nightly branch. It should be easy to fix. > > > > > > Another issue is that IMO, we should bump libdrm's version number when > > > increase these new APIs. Then in Beignet, we can check the libdrm > > > version at build time and determine whether we will use these new > > > interfaces. Thus, we can avoid breaking beignet on those systems which > > > have previous libdrm/kernel installed. > > > > > Right. I can append a libdrm patch to bump the version. And then I suppose I > > will follow the process to make a new release. Not sure right now how that > > works. First time going through it. > > > > Also, how should we test for the libdrm version and conditionally use the > > API? > We can check the libdrm version at configuration time and define a macro to > indicate whether we can use these new APIs in beignet. > > Is there a previous example of this in Beignet that I could follow? > Yes, one example is userptr. You can check the usage of DRM_INTEL_USERPTR and > HAS_USERPTR > In beignet. Ok, applied the kernel patch. Please go ahead with libdrm&beignet parts. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] lib: print a stack trace when a test assertion fails
Add an optional dependency on libunwind to print stack traces when a
test assertion fails.
Signed-off-by: Thomas Wood
---
benchmarks/Makefile.am | 4 ++--
configure.ac | 10 ++
debugger/Makefile.am | 3 ++-
demos/Makefile.am | 4 ++--
lib/Makefile.am| 5 ++---
lib/igt_core.c | 31 +++
tests/Makefile.am | 3 ++-
tools/Makefile.am | 4 ++--
8 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am
index 86f755a..8473b18 100644
--- a/benchmarks/Makefile.am
+++ b/benchmarks/Makefile.am
@@ -2,5 +2,5 @@
include Makefile.sources
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
-AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
-LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS)
+AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS)
+LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS) $(LIBUNWIND_LIBS)
diff --git a/configure.ac b/configure.ac
index 9b646dd..b9ecef8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -187,6 +187,15 @@ AM_CONDITIONAL(BUILD_SHADER_DEBUGGER, [test
"x$BUILD_SHADER_DEBUGGER" != xno])
AS_IF([test "x$BUILD_SHADER_DEBUGGER" != xno],
[enable_debugger=yes], [enable_debugger=no])
+AC_ARG_WITH(libunwind,
+ AS_HELP_STRING([--without-libunwind],
+ [Build tests without libunwind support]),
+ [], [with_libunwind=yes])
+if test "x$with_libunwind" = xyes; then
+ PKG_CHECK_MODULES(LIBUNWIND, libunwind, AC_DEFINE(HAVE_LIBUNWIND, 1,
[libunwind support]),
+ AC_MSG_ERROR([libunwind not found. Use
--without-libunwind to disable libunwind support.]))
+fi
+
# -
# To build multithread code, gcc uses -pthread, Solaris Studio cc uses -mt
@@ -243,6 +252,7 @@ echo ""
echo " • Tests:"
echo " Build tests: ${BUILD_TESTS}"
echo " Compile prime tests: ${NOUVEAU}"
+echo " Print stack traces : ${with_libunwind}"
echo ""
echo " • Tools:"
echo " Assembler : ${enable_assembler}"
diff --git a/debugger/Makefile.am b/debugger/Makefile.am
index f1e49b9..0b6028b 100644
--- a/debugger/Makefile.am
+++ b/debugger/Makefile.am
@@ -12,6 +12,7 @@ AM_CFLAGS = \
$(DRM_CFLAGS) \
$(PCIACCESS_CFLAGS) \
$(CAIRO_CFLAGS) \
+ $(LIBUNWIND_CFLAGS) \
$(CWARNFLAGS)
-LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS)
+LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS) $(LIBUNWIND_LIBS)
diff --git a/demos/Makefile.am b/demos/Makefile.am
index 49804d7..029581a 100644
--- a/demos/Makefile.am
+++ b/demos/Makefile.am
@@ -3,5 +3,5 @@ bin_PROGRAMS = \
$(NULL)
AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
-AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
-LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS)
+AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
$(LIBUNWIND_CFLAGS)
+LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
$(CAIRO_LIBS) $(LIBUNWIND_LIBS)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index a5a4390..4db90d4 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -9,12 +9,11 @@ noinst_LTLIBRARIES = libintel_tools.la
noinst_HEADERS = check-ndebug.h
AM_CPPFLAGS = -I$(top_srcdir)
-AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
+AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(LIBUNWIND_CFLAGS) \
-DIGT_DATADIR=\""$(abs_top_srcdir)/tests"\" \
-DIGT_LOG_DOMAIN=\""$(subst _,-,$*)"\" \
-pthread
-
-LDADD = $(CAIRO_LIBS)
+LDADD = $(CAIRO_LIBS) $(LIBUNWIND_LIBS)
AM_CFLAGS += $(CAIRO_CFLAGS)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 4ae3524..7f879aa 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -955,6 +955,33 @@ static bool run_under_gdb(void)
strncmp(basename(buf), "gdb", 3) == 0);
}
+#ifdef HAVE_LIBUNWIND
+#define UNW_LOCAL_ONLY
+#include
+
+static void print_backtrace(void)
+{
+ unw_cursor_t cursor;
+ unw_context_t uc;
+ int stack_num = 0;
+
+ printf("Stack trace:\n");
+
+ unw_getcontext(&uc);
+ unw_init_local(&cursor, &uc);
+ while (unw_step(&cursor) > 0) {
+ char name[255];
+ unw_word_t off;
+
+ if (unw_get_proc_name(&cursor, name, 255, &off) < 0)
+ strcpy(name, "");
+
+ printf(" #%d [%s+0x%x]\n", stack_num++, name,
+ (unsigned int) off);
+ }
+}
+#endif
+
void __igt_fail_assert(int exitcode, const char *domain, const char *file,
const
Re: [Intel-gfx] [Beignet] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 05:59:13PM +0100, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 09:51:57AM -0700, Jeff McGee wrote: > > On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote: > > > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote: > > > > My only concern is about the following macros: > > > > > > > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33 > > > > > +#define LOCAL_I915_PARAM_EU_TOTAL34 > > > > > > > > How about to just use the definitons in the kernel header file? > > > > For an example: > > > > > > > > #include > > > > > > > > #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL > > > > //Put all the code into this block. > > > > #endif > > > > > > > > Then we can avoid put the same definitions in different files, > > > > and we can avoid unecessary testing on an old kernel which doesn't > > > > have this kernel interface. > > > > > > > > For all the other part, it LGTM. > > > > > > > > Reviewed-by: Zhigang Gong > > > > > > Once we update the libdrm requirements in igt we tend to go around and > > > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra > > > work until then. > > > > > > Patch applied, thanks. > > > -Daniel > > > > > > > Patch applied? Do you want me to make the name change first? Should the > > kernel part be reviewed and merged first? > > Forgot my own review comment already ;-) Fixed up with a follow-up patch. > And I'll pull the kernel part in now too. > -Daniel Thanks. That leaves just the libdrm patch and version bump before Beignet can merge their patches. -Jeff ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'
On Fri, Mar 13, 2015 at 01:32:38PM +, John Harrison wrote:
> On 10/03/2015 10:18, Daniel Vetter wrote:
> >On Mon, Mar 09, 2015 at 11:51:26PM +, Tomas Elf wrote:
> >>On 19/02/2015 17:18, john.c.harri...@intel.com wrote:
> >>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>index 3b0261f..d2c6427 100644
> >>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>@@ -258,10 +258,6 @@ struct intel_engine_cs {
> >>>*/
> >>> struct list_head request_list;
> >>>
> >>>- /**
> >>>- * Do we have some not yet emitted requests outstanding?
> >>>- */
> >>>- struct drm_i915_gem_request *outstanding_lazy_request;
> >>> bool gpu_caches_dirty;
> >>> bool fbc_dirty;
> >>>
> >>>
> >>Since we're removing the i915_add_request from i915_check_olr and thereby
> >>removing the OLR emission the following comment at
> >>i915_gem_object_flush_active is no longer valid:
> >>
> >> /**
> >> * Ensures that an object will eventually get non-busy by flushing
> >>any required
> >> * write domains, emitting any outstanding lazy request and
> >> retiring
> >>and
> >> * completed requests.
> >> */
> >> static int
> >> i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
> >> {
> >> struct intel_engine_cs *ring;
> >> int ret;
> >>
> >> if (obj->active) {
> >> ring =
> >>i915_gem_request_get_ring(obj->last_read_req);
> >> ret = i915_gem_check_olr(obj->last_read_req);
> >Nice catch! On top of that the int return value is no longer needed, so a
> >follow-up patch should simplify it to void.
> >-Daniel
>
> The comment also talks about flushing write domains but there is no obvious
> flush call. All it does is the check_olr (which is now a no-op) and the
> retire (which won't do anything unless the request has already completed).
> So is there any need for this function at all anymore? Or should it just be
> removed and replaced with a call to retire instead?
That's historical baggage - way back we've done lazy cache-flushing, which
mean on readback you might have needed to flush caches first. Massive case
of premature optimization, the comment is just plainly outdated. See
commit 65ce3027415d4dc9ee18ef0a135214b4fb76730b
Author: Chris Wilson
Date: Fri Jul 20 12:41:02 2012 +0100
drm/i915: Remove the defunct flushing list
and
commit cc889e0f6ce6a63c62db17d702ecfed86d58083f
Author: Daniel Vetter
Date: Wed Jun 13 20:45:19 2012 +0200
drm/i915: disable flushing_list/gpu_write_list
You can savely remove the comment (maybe with the above commits referenced
in the commit message).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 04:58:47PM +, Chris Wilson wrote: > On Fri, Mar 13, 2015 at 10:27:38AM +0100, Daniel Vetter wrote: > > If supporting systems without full ppgtt is a requirement for you (still > > wonky on gen8 a bit, so might be a good strategy) then imo it's the > > PIN_BIAS idea I've laid out earlier in this thread. That one will work > > everywhere. softpin can unexpectedly fail without full ppgtt if the kernel > > decides to put something at a given spot, which imo means we should only > > expose it on full ppgtt systems. > > > > And PIN_BIAS should be fairly easy to wire up since the internal logic is > > all there already. So "just" needs an execbuf flag, igt test and > > appropriate userspace to set that new bit. > > It doesn't though. To provide the guarantee userspace is asking for > (which is that address 0 goes to a special, preferrably inaccessible, > page), you have to evict the first N pages in the GGTT. That is just as > likely to fail with an execbuffer flag as it would with an execobject flag. Afaiui userspace only needs the guarantee that NULL is never a valid address. Which means it's never a valid address for its own buffer objects. I don't think it cares one bit what's actually there, it's not mandatory to fault apparently. And faulting is what's not possible. I guess the standard is like normal C: If you access a NULL pointer, anything can happen (including garbage on the frontbuffer), the only guarantee you need to make is that NULL is never a valid address. At least if no one plays tricks ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v5 9/9] drm/i915: Backlight control using CRC PMIC based PWM driver
On Fri, Mar 13, 2015 at 04:30:43PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 12, 2015 at 10:01:33PM +0530, Shobhit Kumar wrote:
> > CC: Samuel Ortiz
> > Cc: Linus Walleij
> > Cc: Alexandre Courbot
> > Cc: Thierry Reding
> > Signed-off-by: Shobhit Kumar
> > ---
> > drivers/gpu/drm/i915/intel_dsi.c | 25 +
> > drivers/gpu/drm/i915/intel_dsi.h | 3 +++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> > b/drivers/gpu/drm/i915/intel_dsi.c
> > index 219421c..511446f 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -32,6 +32,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include "i915_drv.h"
> > #include "intel_drv.h"
> > #include "intel_dsi.h"
> > @@ -402,6 +403,10 @@ static void intel_dsi_enable(struct intel_encoder
> > *encoder)
> >
> > intel_dsi_port_enable(encoder);
> > }
> > +
> > + /* Enable the backlight with default PWM as programmed by BIOS */
> > + pwm_enable(intel_dsi->pwm);
> > + pwm_config_alternate(intel_dsi->pwm, 0x7F, 100);
>
> I suppose we migth want to wrap these in 'if (intel_dsi->pwm)' checks,
> or does the pwm subsystem take care of NULL checks for us?
>
> > }
> >
> > static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> > @@ -467,6 +472,11 @@ static void intel_dsi_pre_disable(struct intel_encoder
> > *encoder)
> >
> > DRM_DEBUG_KMS("\n");
> >
> > + /* Disable the backlight */
> > + pwm_config_alternate(intel_dsi->pwm, 0x7F, 0);
> > + usleep_range(2000, 3000);
> > + pwm_disable(intel_dsi->pwm);
>
> ditto
>
> > +
> > if (is_vid_mode(intel_dsi)) {
> > /* Send Shutdown command to the panel in LP mode */
> > for_each_dsi_port(port, intel_dsi->ports)
> > @@ -971,6 +981,10 @@ static void intel_dsi_encoder_destroy(struct
> > drm_encoder *encoder)
> > if (intel_dsi->gpio_panel)
> > gpiod_put(intel_dsi->gpio_panel);
> >
> > + /* dispose of the pwm */
> > + if (intel_dsi->pwm)
> > + pwm_put(intel_dsi->pwm);
> > +
> > intel_encoder_destroy(encoder);
> > }
> >
> > @@ -1098,6 +1112,17 @@ void intel_dsi_init(struct drm_device *dev)
> > DRM_ERROR("Failed to own gpio for panel control\n");
> > intel_dsi->gpio_panel = NULL;
> > }
> > +
> > + /* Get the PWM chip for backlight control */
> > + intel_dsi->pwm = pwm_get(dev->dev, "pwm_backlight");
> > + if (IS_ERR(intel_dsi->pwm)) {
> > + DRM_ERROR("Faild to own the pwm chip\n");
> > + intel_dsi->pwm = NULL;
If the i915.ko driver loads before the mfd.ko driver we need to do a
deferred probe here I think. Just failing isn't robust.
This also means that if people don't configure their kernel properly then
we'll fall over here, but meh.
-Daniel
> > + } else if (pwm_config_alternate(intel_dsi->pwm, 0x7F, 100) < 0)
> > {
> > + DRM_ERROR("Failed to configure the pwm chip\n");
> > + pwm_put(intel_dsi->pwm);
> > + intel_dsi->pwm = NULL;
> > + }
> > }
> >
> > intel_encoder->type = INTEL_OUTPUT_DSI;
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> > b/drivers/gpu/drm/i915/intel_dsi.h
> > index 8be75a7..d8ec9c1 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.h
> > +++ b/drivers/gpu/drm/i915/intel_dsi.h
> > @@ -52,6 +52,9 @@ struct intel_dsi {
> > /* GPIO Desc for CRC based Panel control */
> > struct gpio_desc *gpio_panel;
> >
> > + /* PWM chip */
> > + struct pwm_device *pwm;
> > +
> > struct intel_connector *attached_connector;
> >
> > /* bit mask of ports being driven */
> > --
> > 2.1.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests: create a single combined test list
On Fri, Mar 13, 2015 at 02:15:46PM +, Thomas Wood wrote:
> All tests now respond in a consistent way such that separate lists for
> tests with and without subtests are no longer necessary.
>
> Signed-off-by: Thomas Wood
Ack on both this and the piglit one. I think we should commit the piglit
patch first, then wait about 1 month or so for ppl to update and then
apply this one here. Afterwards we can also remove the split in
Makefile.sources.
-Daniel
> ---
> tests/Makefile.am | 23 ---
> 1 file changed, 4 insertions(+), 19 deletions(-)
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 5efc8d8..d808973 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -5,32 +5,17 @@ if HAVE_NOUVEAU
> endif
>
> if BUILD_TESTS
> -all-local: single-tests.txt multi-tests.txt
> +all-local: test-list.txt
>
> -list-single-tests:
> - @echo TESTLIST
> - @echo ${single_kernel_tests}
> - @echo END TESTLIST
> -
> -list-multi-tests:
> - @echo TESTLIST
> - @echo ${multi_kernel_tests}
> - @echo END TESTLIST
> -
> -single-tests.txt: Makefile.sources
> - @echo TESTLIST > $@
> - @echo ${single_kernel_tests} >> $@
> - @echo END TESTLIST >> $@
> -
> -multi-tests.txt: Makefile.sources
> +test-list.txt: Makefile.sources
> @echo TESTLIST > $@
> - @echo ${multi_kernel_tests} >> $@
> + @echo ${single_kernel_tests} ${multi_kernel_tests} >> $@
> @echo END TESTLIST >> $@
>
> EXTRA_PROGRAMS = $(TESTS_progs) $(TESTS_progs_M) $(HANG)
> EXTRA_DIST = $(TESTS_scripts) $(TESTS_scripts_M) $(scripts) $(IMAGES)
> $(common_files)
>
> -CLEANFILES = $(EXTRA_PROGRAMS) single-tests.txt multi-tests.txt
> +CLEANFILES = $(EXTRA_PROGRAMS) test-list.txt
>
> AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
> -I$(srcdir)/.. \
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] lib: print a stack trace when a test assertion fails
On Fri, Mar 13, 2015 at 05:04:40PM +, Thomas Wood wrote:
> Add an optional dependency on libunwind to print stack traces when a
> test assertion fails.
>
> Signed-off-by: Thomas Wood
Awesome. Also ack from me (too lazy to dig out manpages on friday evening
for proper review, but looks good).
Cheers, Daniel
> ---
> benchmarks/Makefile.am | 4 ++--
> configure.ac | 10 ++
> debugger/Makefile.am | 3 ++-
> demos/Makefile.am | 4 ++--
> lib/Makefile.am| 5 ++---
> lib/igt_core.c | 31 +++
> tests/Makefile.am | 3 ++-
> tools/Makefile.am | 4 ++--
> 8 files changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am
> index 86f755a..8473b18 100644
> --- a/benchmarks/Makefile.am
> +++ b/benchmarks/Makefile.am
> @@ -2,5 +2,5 @@
> include Makefile.sources
>
> AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
> -AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
> -LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS)
> +AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS)
> +LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS) $(LIBUNWIND_LIBS)
> diff --git a/configure.ac b/configure.ac
> index 9b646dd..b9ecef8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -187,6 +187,15 @@ AM_CONDITIONAL(BUILD_SHADER_DEBUGGER, [test
> "x$BUILD_SHADER_DEBUGGER" != xno])
> AS_IF([test "x$BUILD_SHADER_DEBUGGER" != xno],
>[enable_debugger=yes], [enable_debugger=no])
>
> +AC_ARG_WITH(libunwind,
> + AS_HELP_STRING([--without-libunwind],
> +[Build tests without libunwind support]),
> + [], [with_libunwind=yes])
> +if test "x$with_libunwind" = xyes; then
> + PKG_CHECK_MODULES(LIBUNWIND, libunwind, AC_DEFINE(HAVE_LIBUNWIND, 1,
> [libunwind support]),
> + AC_MSG_ERROR([libunwind not found. Use
> --without-libunwind to disable libunwind support.]))
> +fi
> +
> #
> -
>
> # To build multithread code, gcc uses -pthread, Solaris Studio cc uses -mt
> @@ -243,6 +252,7 @@ echo ""
> echo " • Tests:"
> echo " Build tests: ${BUILD_TESTS}"
> echo " Compile prime tests: ${NOUVEAU}"
> +echo " Print stack traces : ${with_libunwind}"
> echo ""
> echo " • Tools:"
> echo " Assembler : ${enable_assembler}"
> diff --git a/debugger/Makefile.am b/debugger/Makefile.am
> index f1e49b9..0b6028b 100644
> --- a/debugger/Makefile.am
> +++ b/debugger/Makefile.am
> @@ -12,6 +12,7 @@ AM_CFLAGS = \
> $(DRM_CFLAGS) \
> $(PCIACCESS_CFLAGS) \
> $(CAIRO_CFLAGS) \
> + $(LIBUNWIND_CFLAGS) \
> $(CWARNFLAGS)
>
> -LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS)
> +LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS) $(LIBUNWIND_LIBS)
> diff --git a/demos/Makefile.am b/demos/Makefile.am
> index 49804d7..029581a 100644
> --- a/demos/Makefile.am
> +++ b/demos/Makefile.am
> @@ -3,5 +3,5 @@ bin_PROGRAMS =\
> $(NULL)
>
> AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib
> -AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
> -LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS)
> +AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS)
> $(LIBUNWIND_CFLAGS)
> +LDADD = $(top_builddir)/lib/libintel_tools.la $(DRM_LIBS) $(PCIACCESS_LIBS)
> $(CAIRO_LIBS) $(LIBUNWIND_LIBS)
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index a5a4390..4db90d4 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -9,12 +9,11 @@ noinst_LTLIBRARIES = libintel_tools.la
> noinst_HEADERS = check-ndebug.h
>
> AM_CPPFLAGS = -I$(top_srcdir)
> -AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
> +AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(LIBUNWIND_CFLAGS) \
> -DIGT_DATADIR=\""$(abs_top_srcdir)/tests"\" \
> -DIGT_LOG_DOMAIN=\""$(subst _,-,$*)"\" \
> -pthread
>
> -
> -LDADD = $(CAIRO_LIBS)
> +LDADD = $(CAIRO_LIBS) $(LIBUNWIND_LIBS)
> AM_CFLAGS += $(CAIRO_CFLAGS)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 4ae3524..7f879aa 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -955,6 +955,33 @@ static bool run_under_gdb(void)
> strncmp(basename(buf), "gdb", 3) == 0);
> }
>
> +#ifdef HAVE_LIBUNWIND
> +#define UNW_LOCAL_ONLY
> +#include
> +
> +static void print_backtrace(void)
> +{
> + unw_cursor_t cursor;
> + unw_context_t uc;
> + int stack_num = 0;
> +
> + printf("Stack trace:\n");
> +
> + unw_getcontext(&uc);
> + unw_init_local(&cursor, &uc);
> + while (unw_step(&cursor
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 06:13:39PM +0100, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 04:58:47PM +, Chris Wilson wrote: > > On Fri, Mar 13, 2015 at 10:27:38AM +0100, Daniel Vetter wrote: > > > If supporting systems without full ppgtt is a requirement for you (still > > > wonky on gen8 a bit, so might be a good strategy) then imo it's the > > > PIN_BIAS idea I've laid out earlier in this thread. That one will work > > > everywhere. softpin can unexpectedly fail without full ppgtt if the kernel > > > decides to put something at a given spot, which imo means we should only > > > expose it on full ppgtt systems. > > > > > > And PIN_BIAS should be fairly easy to wire up since the internal logic is > > > all there already. So "just" needs an execbuf flag, igt test and > > > appropriate userspace to set that new bit. > > > > It doesn't though. To provide the guarantee userspace is asking for > > (which is that address 0 goes to a special, preferrably inaccessible, > > page), you have to evict the first N pages in the GGTT. That is just as > > likely to fail with an execbuffer flag as it would with an execobject flag. > > Afaiui userspace only needs the guarantee that NULL is never a valid > address. Which means it's never a valid address for its own buffer > objects. I don't think it cares one bit what's actually there, it's not > mandatory to fault apparently. And faulting is what's not possible. You are bending ABI to allow userspace to use absolute addressing (no relocations). The kernel has to make sure that nothing else is at that address. > I guess the standard is like normal C: If you access a NULL pointer, > anything can happen (including garbage on the frontbuffer), the only > guarantee you need to make is that NULL is never a valid address. At least > if no one plays tricks ;-) No. The kernel is quite strict about *NULL. It certainly doesn't allow trivial information leakage. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix vmap_batch page iterator overrun
On Fri, Mar 13, 2015 at 02:05:46PM +, Chris Wilson wrote: > On Fri, Mar 13, 2015 at 03:21:53PM +0200, Mika Kuoppala wrote: > > vmap_batch() calculates amount of needed pages for the mapping > > we are going to create. And it uses this page count as an > > argument for the for_each_sg_pages() macro. The macro takes the number > > of sg list entities as an argument, not the page count. So we ended > > up iterating through all the pages on the mapped object, corrupting > > memory past the smaller pages[] array. > > > > Fix this by bailing out when we have enough pages. Reference to the commit which has introduced this regression is missing, I've added that. Also for next time around pls cc everyone on that patch, especially also reviewers. > > > > Cc: Chris Wilson > > Signed-off-by: Mika Kuoppala > Reviewed-by: Chris Wilson Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 14/13] drm/i915: Unconfuse DP link rate array names
From: Ville Syrjälä
To keep things clear rename the intel_dp->supported_rates[] to
intel_dp->sink_rates[], and rename the supported_rates[] name we used
elsewhere for the intersection of source and sink rates to
common_rates[].
Cc: Sonika Jindal
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 70 ++---
drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h| 5 +--
3 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 715694a..393feed 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1135,9 +1135,9 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state
*pipe_config, int link_bw)
static int
intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
{
- if (intel_dp->num_supported_rates) {
- *sink_rates = intel_dp->supported_rates;
- return intel_dp->num_supported_rates;
+ if (intel_dp->num_sink_rates) {
+ *sink_rates = intel_dp->sink_rates;
+ return intel_dp->num_sink_rates;
}
*sink_rates = default_rates;
@@ -1200,7 +1200,7 @@ intel_dp_set_clock(struct intel_encoder *encoder,
static int intersect_rates(const int *source_rates, int source_len,
const int *sink_rates, int sink_len,
- int *supported_rates)
+ int *common_rates)
{
int i = 0, j = 0, k = 0;
@@ -1208,7 +1208,7 @@ static int intersect_rates(const int *source_rates, int
source_len,
if (source_rates[i] == sink_rates[j]) {
if (WARN_ON(k >= DP_MAX_SUPPORTED_RATES))
return k;
- supported_rates[k] = source_rates[i];
+ common_rates[k] = source_rates[i];
++k;
++i;
++j;
@@ -1221,8 +1221,8 @@ static int intersect_rates(const int *source_rates, int
source_len,
return k;
}
-static int intel_supported_rates(struct intel_dp *intel_dp,
-int *supported_rates)
+static int intel_dp_common_rates(struct intel_dp *intel_dp,
+int *common_rates)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
const int *source_rates, *sink_rates;
@@ -1233,7 +1233,7 @@ static int intel_supported_rates(struct intel_dp
*intel_dp,
return intersect_rates(source_rates, source_len,
sink_rates, sink_len,
- supported_rates);
+ common_rates);
}
static void snprintf_int_array(char *str, size_t len,
@@ -1256,8 +1256,8 @@ static void intel_dp_print_rates(struct intel_dp
*intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
const int *source_rates, *sink_rates;
- int source_len, sink_len, supported_len;
- int supported_rates[DP_MAX_SUPPORTED_RATES];
+ int source_len, sink_len, common_len;
+ int common_rates[DP_MAX_SUPPORTED_RATES];
char str[128]; /* FIXME: too big for stack? */
if ((drm_debug & DRM_UT_KMS) == 0)
@@ -1271,9 +1271,9 @@ static void intel_dp_print_rates(struct intel_dp
*intel_dp)
snprintf_int_array(str, sizeof(str), sink_rates, sink_len);
DRM_DEBUG_KMS("sink rates: %s\n", str);
- supported_len = intel_supported_rates(intel_dp, supported_rates);
- snprintf_int_array(str, sizeof(str), supported_rates, supported_len);
- DRM_DEBUG_KMS("supported rates: %s\n", str);
+ common_len = intel_dp_common_rates(intel_dp, common_rates);
+ snprintf_int_array(str, sizeof(str), common_rates, common_len);
+ DRM_DEBUG_KMS("common rates: %s\n", str);
}
static int rate_to_index(int find, const int *rates)
@@ -1293,7 +1293,7 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
int rates[DP_MAX_SUPPORTED_RATES] = {};
int len;
- len = intel_supported_rates(intel_dp, rates);
+ len = intel_dp_common_rates(intel_dp, rates);
if (WARN_ON(len <= 0))
return 162000;
@@ -1302,7 +1302,7 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
{
- return rate_to_index(rate, intel_dp->supported_rates);
+ return rate_to_index(rate, intel_dp->sink_rates);
}
bool
@@ -1324,15 +1324,15 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
- int supported_rates[DP_MAX_SUPPORTED_RATES] = {};
- int supported_len;
+ int common_rates[DP_MAX_SUPPORTED_RATES] = {};
+ int common_len;
- supported_len = intel_supported_rates(intel_dp, supported_rates);
+ common_len = intel_dp_common_
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
On Fri, Mar 13, 2015 at 05:34:22PM +, Chris Wilson wrote: > On Fri, Mar 13, 2015 at 06:13:39PM +0100, Daniel Vetter wrote: > > On Fri, Mar 13, 2015 at 04:58:47PM +, Chris Wilson wrote: > > > On Fri, Mar 13, 2015 at 10:27:38AM +0100, Daniel Vetter wrote: > > > > If supporting systems without full ppgtt is a requirement for you (still > > > > wonky on gen8 a bit, so might be a good strategy) then imo it's the > > > > PIN_BIAS idea I've laid out earlier in this thread. That one will work > > > > everywhere. softpin can unexpectedly fail without full ppgtt if the > > > > kernel > > > > decides to put something at a given spot, which imo means we should only > > > > expose it on full ppgtt systems. > > > > > > > > And PIN_BIAS should be fairly easy to wire up since the internal logic > > > > is > > > > all there already. So "just" needs an execbuf flag, igt test and > > > > appropriate userspace to set that new bit. > > > > > > It doesn't though. To provide the guarantee userspace is asking for > > > (which is that address 0 goes to a special, preferrably inaccessible, > > > page), you have to evict the first N pages in the GGTT. That is just as > > > likely to fail with an execbuffer flag as it would with an execobject > > > flag. > > > > Afaiui userspace only needs the guarantee that NULL is never a valid > > address. Which means it's never a valid address for its own buffer > > objects. I don't think it cares one bit what's actually there, it's not > > mandatory to fault apparently. And faulting is what's not possible. > > You are bending ABI to allow userspace to use absolute addressing (no > relocations). The kernel has to make sure that nothing else is at that > address. > > > I guess the standard is like normal C: If you access a NULL pointer, > > anything can happen (including garbage on the frontbuffer), the only > > guarantee you need to make is that NULL is never a valid address. At least > > if no one plays tricks ;-) > > No. The kernel is quite strict about *NULL. It certainly doesn't allow > trivial information leakage. We already have that trivial information leakage anyway. Just because we make it a notch more likely that a special address will leak information that doesn't make it different that without full ppgtt you can read any other gpu clients stuff easily. And also write to it if you feel like. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: add psr toggle to debugfs
This patch allows userspace to toggle PSR through a debugfs interface.
It adds functionality to write 0 or 1 to the existing
i915_edp_psr_status file in order to change the relevant module
parameter and enable/disable PSR.
Previous upstream feedback did not like making it a connector property
or putting it in sysfs because that would require API stability going
forward. debugfs interfaces do not have this restriction.
Signed-off-by: Eric Caruso
---
drivers/gpu/drm/i915/i915_debugfs.c | 67 ++---
1 file changed, 63 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 74bc89b..c3d9c89 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2224,10 +2224,9 @@ static int i915_llc(struct seq_file *m, void *data)
return 0;
}
-static int i915_edp_psr_status(struct seq_file *m, void *data)
+static int i915_edp_psr_status_show(struct seq_file *m, void *data)
{
- struct drm_info_node *node = m->private;
- struct drm_device *dev = node->minor->dev;
+ struct drm_device *dev = m->private;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 psrperf = 0;
u32 stat[3];
@@ -2288,6 +2287,66 @@ static int i915_edp_psr_status(struct seq_file *m, void
*data)
return 0;
}
+static int i915_edp_psr_status_open(struct inode *inode, struct file *file)
+{
+ struct drm_device *dev = inode->i_private;
+
+ return single_open(file, i915_edp_psr_status_show, dev);
+}
+
+static ssize_t i915_edp_psr_status_write(struct file *file,
+const char __user *ubuf,
+size_t len, loff_t *offp)
+{
+ struct seq_file *m = file->private_data;
+ struct drm_device *dev = m->private;
+ struct intel_encoder *encoder;
+ struct intel_dp *intel_dp = NULL;
+ int ret;
+ int enable;
+ char tmp[32];
+
+ if (len >= sizeof(tmp))
+ return -EINVAL;
+
+ if (copy_from_user(tmp, ubuf, len))
+ return -EFAULT;
+
+ tmp[len] = '\0';
+
+ ret = kstrtoint(tmp, 0, &enable);
+ if (ret || (enable != 0 && enable != 1))
+ return -EINVAL;
+
+ i915.enable_psr = enable;
+ ret = -ENODEV;
+
+ drm_modeset_lock_all(dev);
+ for_each_intel_encoder(dev, encoder) {
+ if (encoder->type != INTEL_OUTPUT_EDP)
+ continue;
+
+ ret = len;
+ intel_dp = enc_to_intel_dp(&encoder->base);
+
+ if (enable)
+ intel_psr_enable(intel_dp);
+ else
+ intel_psr_disable(intel_dp);
+ }
+ drm_modeset_unlock_all(dev);
+ return ret;
+}
+
+static const struct file_operations i915_edp_psr_status_fops = {
+ .owner = THIS_MODULE,
+ .open = i915_edp_psr_status_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+ .write = i915_edp_psr_status_write
+};
+
static int i915_sink_crc(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -4663,7 +4722,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_swizzle_info", i915_swizzle_info, 0},
{"i915_ppgtt_info", i915_ppgtt_info, 0},
{"i915_llc", i915_llc, 0},
- {"i915_edp_psr_status", i915_edp_psr_status, 0},
{"i915_sink_crc_eDP1", i915_sink_crc, 0},
{"i915_energy_uJ", i915_energy_uJ, 0},
{"i915_pc8_status", i915_pc8_status, 0},
@@ -4698,6 +4756,7 @@ static const struct i915_debugfs_files {
{"i915_spr_wm_latency", &i915_spr_wm_latency_fops},
{"i915_cur_wm_latency", &i915_cur_wm_latency_fops},
{"i915_fbc_false_color", &i915_fbc_fc_fops},
+ {"i915_edp_psr_status", &i915_edp_psr_status_fops},
};
void intel_display_crc_init(struct drm_device *dev)
--
2.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] tests: Remove usage of igt_crc_equal and _non_null
Tests should positively check for crc matches, not for mismatches.
Enforce this by only exposing and igt_assert function for comparing
crcs.
For the few tests which didn't just do this as consistency checks but
to do functional tests add FIXME comments that some reference crc
values are missing.
Signed-off-by: Daniel Vetter
---
lib/igt_debugfs.c | 36 +++-
lib/igt_debugfs.h | 2 --
tests/kms_fbc_crc.c| 6 ++
tests/kms_pipe_crc_basic.c | 12
tests/kms_plane.c | 2 +-
tests/kms_pwrite_crc.c | 4
6 files changed, 6 insertions(+), 56 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 6c350b45..61313aad6ef5 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -62,8 +62,8 @@
* The only way to use #igt_crc_t CRCs therefore is to compare CRCs among each
* another either for equality or difference. Otherwise CRCs must be treated as
* completely opaque values. Note that not even CRCs from different pipes or
tap
- * points on the same platform can be compared. Hence only use
igt_crc_is_null()
- * and igt_assert_crc_equal() to inspect CRC values captured by the same
+ * points on the same platform can be compared. Hence only use
+ * igt_assert_crc_equal() to inspect CRC values captured by the same
* #igt_pipe_crc_t object.
*
* # Other debugfs interface wrappers
@@ -188,14 +188,7 @@ FILE *igt_debugfs_fopen(const char *filename,
* Pipe CRC
*/
-/**
- * igt_crc_is_null:
- * @crc: pipe CRC value to check
- *
- * Returns: True if the CRC is null/invalid, false if it represents a captured
- * valid CRC.
- */
-bool igt_crc_is_null(igt_crc_t *crc)
+static bool igt_crc_is_null(igt_crc_t *crc)
{
int i;
@@ -212,29 +205,6 @@ bool igt_crc_is_null(igt_crc_t *crc)
}
/**
- * igt_crc_equal:
- * @a: first pipe CRC value
- * @b: second pipe CRC value
- *
- * Compares two CRC values.
- *
- * Returns: true if the two CRCs match, false otherwise.
- */
-bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b)
-{
- int i;
-
- if (a->n_words != b->n_words)
- return false;
-
- for (i = 0; i < a->n_words; i++)
- if (a->crc[i] != b->crc[i])
- return false;
-
- return true;
-}
-
-/**
* igt_assert_crc_equal:
* @a: first pipe CRC value
* @b: second pipe CRC value
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index f158c3389fda..ece714863cfe 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -85,8 +85,6 @@ enum intel_pipe_crc_source {
INTEL_PIPE_CRC_SOURCE_MAX,
};
-bool igt_crc_is_null(igt_crc_t *crc);
-bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b);
void igt_assert_crc_equal(igt_crc_t *a, igt_crc_t *b);
char *igt_crc_to_string(igt_crc_t *crc);
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index c42bab9a035f..3491d46bab79 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -267,11 +267,10 @@ static void test_crc(data_t *data, enum test_mode mode)
igt_pipe_crc_start(pipe_crc);
igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
igt_pipe_crc_stop(pipe_crc);
- igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
if (mode == TEST_PAGE_FLIP)
igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
else
- igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+ ;/* FIXME: missing reference CRCs */
free(crcs);
/*
@@ -285,11 +284,10 @@ static void test_crc(data_t *data, enum test_mode mode)
igt_pipe_crc_start(pipe_crc);
igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
igt_pipe_crc_stop(pipe_crc);
- igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
if (mode == TEST_PAGE_FLIP)
igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
else
- igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+ ;/* FIXME: missing reference CRCs */
free(crcs);
}
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 32c08671e5bf..5fc01b19d325 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -168,18 +168,6 @@ test_read_crc_for_output(data_t *data, int pipe,
igt_output_t *output,
igt_debug("CRC for this fb: %s\n", crc_str);
free(crc_str);
- /*
-* make sure the CRC of this fb is different from the ones of
-* previous fbs
-*/
- for (j = 0; j < c; j++)
- igt_assert(!igt_crc_equal(&colors[j].crc,
- &colors[c].crc));
-
- /* ensure the CRCs are not all 0s */
- for (j = 0; j < N_CRCS; j++)
- igt_assert(!igt_crc_is_null(&crcs[j]));
-
/* and ensure that they'are all equal, we haven't changed the
fb */
for (j = 0;
[Intel-gfx] [PATCH 2/5] lib/debugfs: Add igt_assert_crc_equal
Because of hash collisions tests should only ever compare crc
checksums for equality. Checking for inequality can result in random
failures.
To ensure this only expose and igt_assert function and use that.
Follow-up patches will rework the code for tests which don't follow
this requirement and try to compare for CRC inequality.
v2: Rebase on top of Matt's kms_plane changes.
Signed-off-by: Daniel Vetter
---
lib/igt_debugfs.c | 21 -
lib/igt_debugfs.h | 1 +
tests/kms_cursor_crc.c | 4 ++--
tests/kms_fbc_crc.c | 4 ++--
tests/kms_flip_tiling.c | 2 +-
tests/kms_mmio_vs_cs_flip.c | 4 ++--
tests/kms_pipe_crc_basic.c | 2 +-
tests/kms_plane.c | 8
tests/kms_pwrite_crc.c | 2 +-
tests/kms_rotation_crc.c| 4 ++--
tests/kms_universal_plane.c | 14 +++---
11 files changed, 43 insertions(+), 23 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index a2cec45a1460..6c350b45 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -63,7 +63,7 @@
* another either for equality or difference. Otherwise CRCs must be treated as
* completely opaque values. Note that not even CRCs from different pipes or
tap
* points on the same platform can be compared. Hence only use
igt_crc_is_null()
- * and igt_crc_equal() to inspect CRC values captured by the same
+ * and igt_assert_crc_equal() to inspect CRC values captured by the same
* #igt_pipe_crc_t object.
*
* # Other debugfs interface wrappers
@@ -235,6 +235,25 @@ bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b)
}
/**
+ * igt_assert_crc_equal:
+ * @a: first pipe CRC value
+ * @b: second pipe CRC value
+ *
+ * Compares two CRC values and fails the testcase if they don't match with
+ * igt_fail(). Note that due to CRC collisions CRC based testcase can only
+ * assert that CRCs match, never that they are different. Otherwise there might
+ * be random testcase failures when different screen contents end up with the
+ * same CRC by chance.
+ */
+void igt_assert_crc_equal(igt_crc_t *a, igt_crc_t *b)
+{
+ int i;
+
+ for (i = 0; i < a->n_words; i++)
+ igt_assert_eq_u32(a->crc[i], b->crc[i]);
+}
+
+/**
* igt_crc_to_string:
* @crc: pipe CRC value to print
*
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 828502957a98..f158c3389fda 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -87,6 +87,7 @@ enum intel_pipe_crc_source {
bool igt_crc_is_null(igt_crc_t *crc);
bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b);
+void igt_assert_crc_equal(igt_crc_t *a, igt_crc_t *b);
char *igt_crc_to_string(igt_crc_t *crc);
void igt_require_pipe_crc(void);
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 3de7e021f3f3..0bfe15ee03d4 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -134,7 +134,7 @@ static void do_single_test(data_t *data, int x, int y)
igt_wait_for_vblank(data->drm_fd, data->pipe);
igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
/* Clear screen afterwards */
- igt_assert(igt_crc_equal(&crc, &ref_crc));
+ igt_assert_crc_equal(&crc, &ref_crc);
igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
0.0, 0.0, 0.0);
@@ -451,7 +451,7 @@ static void test_cursor_size(data_t *data)
/* Clear screen afterwards */
igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
0.0, 0.0, 0.0);
- igt_assert(igt_crc_equal(&crc[i], &ref_crc));
+ igt_assert_crc_equal(&crc[i], &ref_crc);
}
}
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 32c3d9899d1a..c42bab9a035f 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -269,7 +269,7 @@ static void test_crc(data_t *data, enum test_mode mode)
igt_pipe_crc_stop(pipe_crc);
igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
if (mode == TEST_PAGE_FLIP)
- igt_assert(igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+ igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
else
igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
free(crcs);
@@ -287,7 +287,7 @@ static void test_crc(data_t *data, enum test_mode mode)
igt_pipe_crc_stop(pipe_crc);
igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[0]));
if (mode == TEST_PAGE_FLIP)
- igt_assert(igt_crc_equal(&crcs[0], &data->ref_crc[1]));
+ igt_assert_crc_equal(&crcs[0], &data->ref_crc[1]);
else
igt_assert(!igt_crc_equal(&crcs[0], &data->ref_crc[1]));
free(crcs);
diff --git a/tests/kms_flip_tiling.c b/tests/kms_flip_tiling.c
index 32f167ab1771..c3d7bcbfe429 100644
--- a/tests/kms_flip_tiling.c
+++ b/tests/kms_flip_tiling.c
@@ -110,7 +110,7 @@ test_flip_changes_tiling(data_t *data, igt_output_t *output)
/* get a crc and compare with the re
[Intel-gfx] [PATCH 5/5] tests/kms_cursor_crc: Add dpms and suspend tests
There was some confusion about whether we restore cursors correctly
after dpms and suspend/resume. Apparently we still do!
Signed-off-by: Daniel Vetter
---
tests/kms_cursor_crc.c | 42 --
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 0bfe15ee03d4..d33ee932d673 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -63,8 +63,12 @@ typedef struct {
int cursor_max_w, cursor_max_h;
igt_pipe_crc_t *pipe_crc;
uint32_t devid;
+ unsigned flags;
} data_t;
+#define TEST_DPMS (1<<0)
+#define TEST_SUSPEND (1<<1)
+
static void draw_cursor(cairo_t *cr, int x, int y, int cw, int ch)
{
int wl, wr, ht, hb;
@@ -124,6 +128,27 @@ static void do_single_test(data_t *data, int x, int y)
igt_display_commit(display);
igt_wait_for_vblank(data->drm_fd, data->pipe);
igt_pipe_crc_collect_crc(pipe_crc, &crc);
+
+ if (data->flags & (TEST_DPMS | TEST_SUSPEND)) {
+ igt_crc_t crc_after;
+
+ if (data->flags & TEST_DPMS) {
+ igt_debug("dpms off/on cycle\n");
+ kmstest_set_connector_dpms(data->drm_fd,
+
data->output->config.connector,
+ DRM_MODE_DPMS_OFF);
+ kmstest_set_connector_dpms(data->drm_fd,
+
data->output->config.connector,
+ DRM_MODE_DPMS_ON);
+ }
+
+ if (data->flags & TEST_SUSPEND)
+ igt_system_suspend_autoresume();
+
+ igt_pipe_crc_collect_crc(pipe_crc, &crc_after);
+ igt_assert_crc_equal(&crc, &crc_after);
+ }
+
cursor_disable(data);
igt_display_commit(display);
@@ -248,10 +273,12 @@ static void test_crc_sliding(data_t *data)
static void test_crc_random(data_t *data)
{
- int i;
+ int i, max;
+
+ max = data->flags & (TEST_DPMS | TEST_SUSPEND) ? 2 : 50;
/* Random cursor placement */
- for (i = 0; i < 50; i++) {
+ for (i = 0; i < max; i++) {
int x = rand() % (data->screenw + data->curw * 2) - data->curw;
int y = rand() % (data->screenh + data->curh * 2) - data->curh;
do_single_test(data, x, y);
@@ -474,6 +501,17 @@ static void run_test_generic(data_t *data)
run_test(data, test_crc_sliding, w, h);
igt_subtest_f("cursor-%dx%d-random", w, h)
run_test(data, test_crc_random, w, h);
+ igt_subtest_f("cursor-%dx%d-dpms", w, h) {
+ data->flags = TEST_DPMS;
+ run_test(data, test_crc_random, w, h);
+ data->flags = 0;
+ }
+
+ igt_subtest_f("cursor-%dx%d-suspend", w, h) {
+ data->flags = TEST_SUSPEND;
+ run_test(data, test_crc_random, w, h);
+ data->flags = 0;
+ }
igt_fixture
igt_remove_fb(data->drm_fd, &data->fb);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] lib: Only warn about suspicious CRCs
It is theoretically possible to hit these in the wild, so only warn
about them. Dropping the test is probably too much since these caught
some real bugs in the past.
Signed-off-by: Daniel Vetter
---
lib/igt_debugfs.c | 35 ++-
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 61313aad6ef5..85c3f22293cc 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -188,22 +188,6 @@ FILE *igt_debugfs_fopen(const char *filename,
* Pipe CRC
*/
-static bool igt_crc_is_null(igt_crc_t *crc)
-{
- int i;
-
- for (i = 0; i < crc->n_words; i++) {
- igt_warn_on_f(crc->crc[i] == 0x,
- "Suspicious CRC: it looks like the CRC "
- "read back was from a register in a powered "
- "down well\n");
- if (crc->crc[i])
- return false;
- }
-
- return true;
-}
-
/**
* igt_assert_crc_equal:
* @a: first pipe CRC value
@@ -498,6 +482,23 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
*out_crcs = crcs;
}
+static void crc_sanity_checks(igt_crc_t *crc)
+{
+ int i;
+ bool all_zero = true;
+
+ for (i = 0; i < crc->n_words; i++) {
+ igt_warn_on_f(crc->crc[i] == 0x,
+ "Suspicious CRC: it looks like the CRC "
+ "read back was from a register in a powered "
+ "down well\n");
+ if (crc->crc[i])
+ all_zero = false;
+ }
+
+ igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
+}
+
/**
* igt_pipe_crc_collect_crc:
* @pipe_crc: pipe CRC object
@@ -515,7 +516,7 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc,
igt_crc_t *out_crc)
read_one_crc(pipe_crc, out_crc);
igt_pipe_crc_stop(pipe_crc);
- igt_assert(!igt_crc_is_null(out_crc));
+ crc_sanity_checks(out_crc);
}
/*
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] lib/core: add interactive debug point to igt_fail
Useful for inspecting the screen state in kms tests when they fail.
Also move the screen clearing in kms_cursor_crc to the bottom.
Signed-off-by: Daniel Vetter
---
lib/igt_core.c | 2 ++
tests/kms_cursor_crc.c | 5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index c217a016f856..410554435b70 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -917,6 +917,8 @@ void igt_fail(int exitcode)
{
assert(exitcode != IGT_EXIT_SUCCESS && exitcode != IGT_EXIT_SKIP);
+ igt_debug_wait_for_keypress("failure");
+
if (!failed_one)
igt_exitcode = exitcode;
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 94e2b4a731e6..3de7e021f3f3 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -130,13 +130,14 @@ static void do_single_test(data_t *data, int x, int y)
/* Now render the same in software and collect crc */
draw_cursor(cr, x, y, data->curw, data->curh);
igt_display_commit(display);
+
igt_wait_for_vblank(data->drm_fd, data->pipe);
igt_pipe_crc_collect_crc(pipe_crc, &ref_crc);
/* Clear screen afterwards */
+ igt_assert(igt_crc_equal(&crc, &ref_crc));
+
igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
0.0, 0.0, 0.0);
-
- igt_assert(igt_crc_equal(&crc, &ref_crc));
}
static void do_fail_test(data_t *data, int x, int y, int expect)
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add psr toggle to debugfs
On Fri, Mar 13, 2015 at 7:10 PM, Eric Caruso wrote: > This patch allows userspace to toggle PSR through a debugfs interface. > It adds functionality to write 0 or 1 to the existing > i915_edp_psr_status file in order to change the relevant module > parameter and enable/disable PSR. > > Previous upstream feedback did not like making it a connector property > or putting it in sysfs because that would require API stability going > forward. debugfs interfaces do not have this restriction. > > Signed-off-by: Eric Caruso What do we need this for? debugfs is generally for debugging (where the module option should be enough) and for testcases (which doesn't seem to be the case here). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add psr toggle to debugfs
2015-03-13 16:01 GMT-03:00 Daniel Vetter : > On Fri, Mar 13, 2015 at 7:10 PM, Eric Caruso wrote: >> This patch allows userspace to toggle PSR through a debugfs interface. >> It adds functionality to write 0 or 1 to the existing >> i915_edp_psr_status file in order to change the relevant module >> parameter and enable/disable PSR. >> >> Previous upstream feedback did not like making it a connector property >> or putting it in sysfs because that would require API stability going >> forward. debugfs interfaces do not have this restriction. >> >> Signed-off-by: Eric Caruso > > What do we need this for? debugfs is generally for debugging (where > the module option should be enough) and for testcases (which doesn't > seem to be the case here). What is wrong with "echo 1 > /sys/module/i915/parameters/enable_psr"? > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add psr toggle to debugfs
On Fri, Mar 13, 2015 at 1:14 PM, Paulo Zanoni wrote: > 2015-03-13 16:01 GMT-03:00 Daniel Vetter : >> On Fri, Mar 13, 2015 at 7:10 PM, Eric Caruso wrote: >>> This patch allows userspace to toggle PSR through a debugfs interface. >>> It adds functionality to write 0 or 1 to the existing >>> i915_edp_psr_status file in order to change the relevant module >>> parameter and enable/disable PSR. >>> >>> Previous upstream feedback did not like making it a connector property >>> or putting it in sysfs because that would require API stability going >>> forward. debugfs interfaces do not have this restriction. >>> >>> Signed-off-by: Eric Caruso >> >> What do we need this for? debugfs is generally for debugging (where >> the module option should be enough) and for testcases (which doesn't >> seem to be the case here). > > What is wrong with "echo 1 > /sys/module/i915/parameters/enable_psr"? > >> -Daniel >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> ___ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni Forgive me if this is wrong, but updating the module parameter doesn't change whether or not PSR is enabled immediately. You need to go through intel_enable_ddi for this to take effect. We were using something like this for testing on Chromium OS, because PSR would sometimes cause issues. This allows us to change the module parameter and enable/disable PSR in one step. The upstream feedback I mentioned was done about a year ago: http://lists.freedesktop.org/archives/intel-gfx/2014-March/041896.html It seemed to favor a debugfs knob for this. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Updated drm-intel-testing
Hi all, New -testing cycle with cool stuff: - EU count report param for gen9+ (Jeff McGee) - piles of pll/wm/... fixes for chv, finally out of preliminary hw support (Ville, Vijay) - gen9 rps support from Akash - more work to move towards atomic from Matt, Ander and others - runtime pm support for skl (Damien) - edp1.4 intermediate link clock support (Sonika) - use frontbuffer tracking for fbc (Paulo) - remove ilk rc6 (John Harrison) - a bunch of smaller things and fixes all over Happy testing! Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 03/10] tests/kms_psr_sink_crc: Make mmaps visible to human eyes
this will allow manual tests when crc isn't available.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 5085fb3..dbc09e4 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -83,6 +83,7 @@ typedef struct {
drm_intel_bufmgr *bufmgr;
struct igt_fb fb_green, fb_white;
igt_plane_t *primary, *sprite, *cursor;
+ int mod_size;
} data_t;
static void create_cursor_fb(data_t *data)
@@ -350,19 +351,21 @@ static void test_crc(data_t *data)
igt_assert(is_green(crc));
break;
case MMAP_GTT:
- ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+ ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+ PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0xcc, data->mod_size);
+ munmap(ptr, data->mod_size);
break;
case MMAP_GTT_WAITING:
- ptr = gem_mmap__gtt(data->drm_fd, handle, 4096, PROT_WRITE);
+ ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
+ PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
/* Printing white on white so the screen shouldn't change */
- memset(ptr, 0xff, 4);
+ memset(ptr, 0xff, data->mod_size);
get_sink_crc(data, crc);
igt_assert(strcmp(ref_crc, crc) == 0);
@@ -370,15 +373,15 @@ static void test_crc(data_t *data)
sleep(10);
/* Now lets print black to change the screen */
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0, data->mod_size);
+ munmap(ptr, data->mod_size);
break;
case MMAP_CPU:
- ptr = gem_mmap__cpu(data->drm_fd, handle, 0, 4096, PROT_WRITE);
+ ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size,
PROT_WRITE);
gem_set_domain(data->drm_fd, handle,
I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
- memset(ptr, 0, 4);
- munmap(ptr, 4096);
+ memset(ptr, 0, data->mod_size);
+ munmap(ptr, data->mod_size);
gem_sw_finish(data->drm_fd, handle);
break;
case BLT:
@@ -447,6 +450,9 @@ static void run_test(data_t *data)
white_h = mode->hdisplay;
white_v = mode->vdisplay;
+ /* Ignoring pitch and bpp to avoid changing full screen */
+ data->mod_size = white_h * white_v;
+
switch (data->test_plane) {
case SPRITE:
data->sprite = igt_output_get_plane(output,
@@ -471,6 +477,9 @@ static void run_test(data_t *data)
igt_plane_set_fb(data->cursor, NULL);
create_cursor_fb(data);
igt_plane_set_position(data->cursor, 0, 0);
+
+ /* Cursor is 64 x 64, ignoring pitch and bbp again */
+ data->mod_size = 64 * 64;
break;
}
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 07/10] tests/kms_psr_sink_crc: Use pressed key to pass/fail.
This is useful when 1 person is running all tests and other one is reading log
willing
to know what tests passed and which failed. So tester is able to run all tests
without
stop and send log to developer.
v2: Rebased after igt_debug_warn_and_wait_for_key
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 24f5ca8..ce50cdd 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -316,11 +316,21 @@ static bool is_green(char *crc)
static void assert_or_manual(bool condition, const char *expected)
{
- if (igt_interactive_debug)
- igt_info("Is %s?\n", expected);
- else
- igt_debug("%s\n", expected);
- igt_debug_wait_for_keypress("manual");
+ char msg[50];
+ char c;
+
+ igt_debug("%s\n", expected);
+
+ sprintf(msg, "Is %s [Y/n]? ", expected);
+ c = igt_debug_warn_and_wait_for_key("manual", msg);
+
+ if (c) {
+ if (c == 'n' || c == 'N')
+ igt_fail(-1);
+ else
+ igt_info("\n");
+ }
+
igt_assert(igt_interactive_debug || condition);
}
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 06/10] lib/igt_aux: Introduce igt_debug_warn_and_wait_for_key.
This is an extention of igt_debug_wait_for_keypress that also can have
customized message and return key pressed.
v2: This is actualy a v2. V1 was an extension of original
igt_debug_wait_for_keypress but it was nacked.
Signed-off-by: Rodrigo Vivi
---
lib/igt_aux.c | 46 ++
lib/igt_aux.h | 1 +
2 files changed, 47 insertions(+)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 131ff4b..0783ac2 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -448,6 +448,52 @@ void igt_debug_wait_for_keypress(const char *var)
tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
}
+/**
+ * igt_debug_wait_for_key:
+ * @var: var lookup to to enable this wait
+ * @msg: message to be printed before wait for key
+ *
+ * Waits for a key press when run interactively and when the corresponding
debug
+ * var is set in the --interactive-debug= variable. Multiple keys
+ * can be specified as a comma-separated list or alternatively "all" if a wait
+ * should happen for all cases.
+ *
+ * When not connected to a terminal interactive_debug is ignored
+ * and execution immediately continues.
+ *
+ * This is useful for display tests where under certain situation manual
+ * inspection of the display is useful. Or when running a testcase in the
+ * background.
+ *
+ * Returns: key pressed block
+ */
+char igt_debug_warn_and_wait_for_key(const char *var, const char *msg)
+{
+ struct termios oldt, newt;
+ char key = 0;
+
+ if (!isatty(STDIN_FILENO))
+ return key;
+
+ if (!igt_interactive_debug)
+ return key;
+
+ if (!strstr(igt_interactive_debug, var) &&
+ !strstr(igt_interactive_debug, "all"))
+ return key;
+
+ igt_info("%s", msg);
+
+ tcgetattr ( STDIN_FILENO, &oldt );
+ newt = oldt;
+ newt.c_lflag &= ~ICANON;
+ tcsetattr ( STDIN_FILENO, TCSANOW, &newt );
+ key = getchar();
+ tcsetattr ( STDIN_FILENO, TCSANOW, &oldt );
+
+ return key;
+}
+
#define POWER_DIR "/sys/devices/pci:00/:00:02.0/power"
/* We just leak this on exit ... */
int pm_status_fd = -1;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 0c361f2..146712c 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -63,6 +63,7 @@ void igt_system_hibernate_autoresume(void);
void igt_drop_root(void);
void igt_debug_wait_for_keypress(const char *var);
+char igt_debug_warn_and_wait_for_key(const char *var, const char *msg);
enum igt_runtime_pm_status {
IGT_RUNTIME_PM_STATUS_ACTIVE,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 10/10] test/kms_psr_sink_crc: Add dpms off/on tests.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 8d09b0a..38013df 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -539,6 +539,15 @@ static void setup_test_plane(data_t *data)
igt_display_commit(&data->display);
}
+static void dpms_off_on(data_t data)
+{
+ kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+ DRM_MODE_DPMS_OFF);
+ sleep(1);
+ kmstest_set_connector_dpms(data.drm_fd, data.output->config.connector,
+ DRM_MODE_DPMS_ON);
+}
+
static int opt_handler(int opt, int opt_index)
{
switch (opt) {
@@ -614,6 +623,30 @@ int main(int argc, char *argv[])
}
}
+ igt_subtest_f("dpms_off_psr_active") {
+ data.test_plane = PRIMARY;
+ data.op = RENDER;
+ setup_test_plane(&data);
+ igt_assert(wait_psr_entry(&data));
+
+ dpms_off_on(data);
+
+ run_test(&data);
+ test_cleanup(&data);
+ }
+
+ igt_subtest_f("dpms_off_psr_exit") {
+ data.test_plane = SPRITE;
+ data.op = PLANE_ONOFF;
+ setup_test_plane(&data);
+
+ dpms_off_on(data);
+
+ igt_assert(wait_psr_entry(&data));
+ run_test(&data);
+ test_cleanup(&data);
+ }
+
igt_fixture {
drm_intel_bufmgr_destroy(data.bufmgr);
display_fini(&data);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 08/10] tests/kms_psr_sink_crc: remove timeout option from wait_psr_entry.
No functional changes. Just making timeout unique for any case.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index ce50cdd..733083a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -245,8 +245,9 @@ static bool psr_active(data_t *data)
return strcmp(str, "yes") == 0;
}
-static bool wait_psr_entry(data_t *data, int timeout)
+static bool wait_psr_entry(data_t *data)
{
+ int timeout = 10;
while (timeout--) {
if (psr_active(data))
return true;
@@ -351,7 +352,7 @@ static void test_crc(data_t *data)
assert_or_manual(is_green(ref_crc), "screen GREEN");
/* Confirm screen stays Green after PSR got active */
- igt_assert(wait_psr_entry(data, 10));
+ igt_assert(wait_psr_entry(data));
get_sink_crc(data, ref_crc);
assert_or_manual(is_green(ref_crc), "screen GREEN");
@@ -365,7 +366,7 @@ static void test_crc(data_t *data)
igt_display_commit(&data->display);
/* Confirm it is not Green anymore */
- igt_assert(wait_psr_entry(data, 10));
+ igt_assert(wait_psr_entry(data));
get_sink_crc(data, ref_crc);
if (data->test_plane == PRIMARY)
assert_or_manual(!is_green(ref_crc), "screen WHITE");
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 01/10] tests/kms_psr_sink_crc: Make blt visible to human eyes
This will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index ba6fb1d..0a56705 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -123,7 +123,7 @@ static void fill_blt(data_t *data, uint32_t handle, unsigned char color) COLOR_BLIT_COPY_BATCH_START(0); OUT_BATCH((1 << 24) | (0xf0 << 16) | 0); OUT_BATCH(0); - OUT_BATCH(1 << 16 | 4); + OUT_BATCH(0xfff << 16 | 0xfff); OUT_RELOC(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0); OUT_BATCH(color); ADVANCE_BATCH(); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 04/10] tests/kms_psr_sink_crc: Make plane_move visible to human eyes
this will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index dbc09e4..b7873b4 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -392,7 +392,7 @@ static void test_crc(data_t *data) break; case PLANE_MOVE: /* Only in use when testing Sprite and Cursor */ - igt_plane_set_position(test_plane, 1, 1); + igt_plane_set_position(test_plane, 500, 500); igt_display_commit(&data->display); break; case PLANE_ONOFF: -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 09/10] test/kms_psr_sink_crc: Split plane setup operations
No functional changes. This reorg will allow to do some
operations like dpms off/on with different places to wait
for psr to get active.
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 138 ++-
1 file changed, 76 insertions(+), 62 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 733083a..8d09b0a 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -84,6 +84,8 @@ typedef struct {
struct igt_fb fb_green, fb_white;
igt_plane_t *primary, *sprite, *cursor;
int mod_size;
+ drmModeModeInfo *mode;
+ igt_output_t *output;
} data_t;
static void create_cursor_fb(data_t *data)
@@ -101,9 +103,32 @@ static void create_cursor_fb(data_t *data)
igt_assert(cairo_status(cr) == 0);
}
+
+static void setup_output(data_t *data)
+{
+ igt_display_t *display = &data->display;
+ igt_output_t *output;
+
+ for_each_connected_output(display, output) {
+ drmModeConnectorPtr c = output->config.connector;
+
+ if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
+ c->connection != DRM_MODE_CONNECTED)
+ continue;
+
+ igt_output_set_pipe(output, PIPE_ANY);
+ data->crtc_id = output->config.crtc->crtc_id;
+ data->output = output;
+ data->mode = igt_output_get_mode(output);
+
+ return;
+ }
+}
+
static void display_init(data_t *data)
{
igt_display_init(&data->display, data->drm_fd);
+ setup_output(data);
}
static void display_fini(data_t *data)
@@ -335,7 +360,7 @@ static void assert_or_manual(bool condition, const char
*expected)
igt_assert(igt_interactive_debug || condition);
}
-static void test_crc(data_t *data)
+static void run_test(data_t *data)
{
uint32_t handle = data->fb_white.gem_handle;
igt_plane_t *test_plane;
@@ -344,9 +369,6 @@ static void test_crc(data_t *data)
char crc[12];
const char *expected = "";
- igt_plane_set_fb(data->primary, &data->fb_green);
- igt_display_commit(&data->display);
-
/* Confirm that screen became Green */
get_sink_crc(data, ref_crc);
assert_or_manual(is_green(ref_crc), "screen GREEN");
@@ -461,77 +483,60 @@ static void test_cleanup(data_t *data) {
igt_remove_fb(data->drm_fd, &data->fb_white);
}
-static void run_test(data_t *data)
+static void setup_test_plane(data_t *data)
{
- igt_display_t *display = &data->display;
- igt_output_t *output;
- drmModeModeInfo *mode;
uint32_t white_h, white_v;
- for_each_connected_output(display, output) {
- drmModeConnectorPtr c = output->config.connector;
+ igt_create_color_fb(data->drm_fd,
+ data->mode->hdisplay, data->mode->vdisplay,
+ DRM_FORMAT_XRGB,
+ LOCAL_I915_FORMAT_MOD_X_TILED,
+ 0.0, 1.0, 0.0,
+ &data->fb_green);
- if (c->connector_type != DRM_MODE_CONNECTOR_eDP ||
- c->connection != DRM_MODE_CONNECTED)
- continue;
+ data->primary = igt_output_get_plane(data->output, IGT_PLANE_PRIMARY);
+ igt_plane_set_fb(data->primary, NULL);
- igt_output_set_pipe(output, PIPE_ANY);
- data->crtc_id = output->config.crtc->crtc_id;
+ white_h = data->mode->hdisplay;
+ white_v = data->mode->vdisplay;
- mode = igt_output_get_mode(output);
+ /* Ignoring pitch and bpp to avoid changing full screen */
+ data->mod_size = white_h * white_v;
+ switch (data->test_plane) {
+ case SPRITE:
+ data->sprite = igt_output_get_plane(data->output,
+ IGT_PLANE_2);
+ igt_plane_set_fb(data->sprite, NULL);
+ /* To make it different for human eyes let's make
+* sprite visible in only one quarter of the primary
+*/
+ white_h = white_h/2;
+ white_v = white_v/2;
+ case PRIMARY:
igt_create_color_fb(data->drm_fd,
- mode->hdisplay, mode->vdisplay,
+ white_h, white_v,
DRM_FORMAT_XRGB,
LOCAL_I915_FORMAT_MOD_X_TILED,
- 0.0, 1.0, 0.0,
- &data->fb_green);
-
- data->primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
- igt_plane_set_fb(data->primary, NULL);
-
- white_h = mode->hdisplay;
- white_v = mode->vdisplay;
-
- /* Ignoring pitch and bpp to avoid changing full screen */
- data->mod
[Intel-gfx] [PATCH i-g-t 05/10] tests/kms_psr_sink_crc: Add manual mode.
Sink CRC is the most reliable way to test PSR. However in some platforms
apparently auto generated packages force panel to keep calculating CRC
invalidating
our current sink crc check over debugfs.
So, this manual test help us to find possible gaps on this platforms where we
cannot
trust on sink crc checks.
v2: Accept Daniel's suggestions:
* Avoid strcpy
* don't override assert definition
* Make --interactive-debug for every testcases instead using local --manual
v3: Sink CRC can be unreliable for other platforms as well so let's skip and
warn
when we detect the misbehaviour instead hardcoded per platform.
Cc: Daniel Vetter
Signed-off-by: Rodrigo Vivi
---
tests/kms_psr_sink_crc.c | 46 +++---
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index b7873b4..24f5ca8 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -259,11 +259,14 @@ static void get_sink_crc(data_t *data, char *crc) {
int ret;
FILE *file;
+ if (igt_interactive_debug)
+ return;
+
file = igt_debugfs_fopen("i915_sink_crc_eDP1", "r");
igt_require(file);
ret = fscanf(file, "%s\n", crc);
- igt_require(ret > 0);
+ igt_require_f(ret > 0, "Sink CRC is unreliable on this machine. Try
manual debug with --interactive-debug=manual\n");
fclose(file);
@@ -286,6 +289,9 @@ static bool is_green(char *crc)
unsigned int rh, gh, bh, mask;
int ret;
+ if (igt_interactive_debug)
+ return false;
+
sscanf(color_mask, "%4x", &mask);
memcpy(rs, &crc[0], 4);
@@ -308,6 +314,16 @@ static bool is_green(char *crc)
(bh & mask) == 0);
}
+static void assert_or_manual(bool condition, const char *expected)
+{
+ if (igt_interactive_debug)
+ igt_info("Is %s?\n", expected);
+ else
+ igt_debug("%s\n", expected);
+ igt_debug_wait_for_keypress("manual");
+ igt_assert(igt_interactive_debug || condition);
+}
+
static void test_crc(data_t *data)
{
uint32_t handle = data->fb_white.gem_handle;
@@ -315,18 +331,19 @@ static void test_crc(data_t *data)
void *ptr;
char ref_crc[12];
char crc[12];
+ const char *expected = "";
igt_plane_set_fb(data->primary, &data->fb_green);
igt_display_commit(&data->display);
/* Confirm that screen became Green */
get_sink_crc(data, ref_crc);
- igt_assert(is_green(ref_crc));
+ assert_or_manual(is_green(ref_crc), "screen GREEN");
/* Confirm screen stays Green after PSR got active */
igt_assert(wait_psr_entry(data, 10));
get_sink_crc(data, ref_crc);
- igt_assert(is_green(ref_crc));
+ assert_or_manual(is_green(ref_crc), "screen GREEN");
/* Setting a secondary fb/plane */
switch (data->test_plane) {
@@ -340,7 +357,10 @@ static void test_crc(data_t *data)
/* Confirm it is not Green anymore */
igt_assert(wait_psr_entry(data, 10));
get_sink_crc(data, ref_crc);
- igt_assert(!is_green(ref_crc));
+ if (data->test_plane == PRIMARY)
+ assert_or_manual(!is_green(ref_crc), "screen WHITE");
+ else
+ assert_or_manual(!is_green(ref_crc), "GREEN background with
WHITE box");
switch (data->op) {
case PAGE_FLIP:
@@ -348,7 +368,8 @@ static void test_crc(data_t *data)
igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
data->fb_green.fb_id, 0, NULL) == 0);
get_sink_crc(data, crc);
- igt_assert(is_green(crc));
+ assert_or_manual(is_green(crc), "screen GREEN");
+ expected = "still GREEN";
break;
case MMAP_GTT:
ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -357,6 +378,7 @@ static void test_crc(data_t *data)
I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
memset(ptr, 0xcc, data->mod_size);
munmap(ptr, data->mod_size);
+ expected = "BLACK or TRANSPARENT mark on top of plane in test";
break;
case MMAP_GTT_WAITING:
ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
@@ -367,7 +389,11 @@ static void test_crc(data_t *data)
/* Printing white on white so the screen shouldn't change */
memset(ptr, 0xff, data->mod_size);
get_sink_crc(data, crc);
- igt_assert(strcmp(ref_crc, crc) == 0);
+ if (data->test_plane == PRIMARY)
+ assert_or_manual(strcmp(ref_crc, crc) == 0, "screen
WHITE");
+ else
+ assert_or_manual(strcmp(ref_crc, crc) == 0,
+ "GREEN background wit
[Intel-gfx] [PATCH i-g-t 02/10] tests/kms_psr_sink_crc: Make render visible to human eyes
This will allow manual tests when crc isn't available. Signed-off-by: Rodrigo Vivi --- tests/kms_psr_sink_crc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c index 0a56705..5085fb3 100644 --- a/tests/kms_psr_sink_crc.c +++ b/tests/kms_psr_sink_crc.c @@ -139,7 +139,7 @@ static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo) buf->bo = bo; buf->stride = 4096; buf->tiling = I915_TILING_X; - buf->size = 4096; + buf->size = 4; } static void fill_render(data_t *data, uint32_t handle, unsigned char color) @@ -167,7 +167,7 @@ static void fill_render(data_t *data, uint32_t handle, unsigned char color) igt_assert(batch); rendercopy(batch, NULL, - &src_buf, 0, 0, 1, 1, + &src_buf, 0, 0, 0xff, 0xff, &dst_buf, 0, 0); intel_batchbuffer_free(batch); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/13] drm/i915: Make the DP rates int instead of uint32_t
On 3/12/2015 8:10 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
No point in using uint32_t here, just plain old int will do.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 33d5877..1fa8cc1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -85,10 +85,9 @@ static const struct dp_link_dpll chv_dpll[] = {
{ .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
};
/* Skylake supports following rates */
-static const uint32_t gen9_rates[] = { 162000, 216000, 27, 324000,
- 432000, 54 };
-
-static const uint32_t default_rates[] = { 162000, 27, 54 };
+static const int gen9_rates[] = { 162000, 216000, 27,
+ 324000, 432000, 54 };
+static const int default_rates[] = { 162000, 27, 54 };
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
@@ -1139,7 +1138,7 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state
*pipe_config, int link_bw)
}
static int
-intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t *sink_rates)
+intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
int i = 0;
@@ -1166,7 +1165,7 @@ intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t
*sink_rates)
}
static int
-intel_read_source_rates(struct intel_dp *intel_dp, uint32_t *source_rates)
+intel_read_source_rates(struct intel_dp *intel_dp, int *source_rates)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
int i;
@@ -1217,8 +1216,9 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
-static int intel_supported_rates(const uint32_t *source_rates, int source_len,
-const uint32_t *sink_rates, int sink_len, uint32_t *supported_rates)
+static int intel_supported_rates(const int *source_rates, int source_len,
+const int *sink_rates, int sink_len,
+int *supported_rates)
{
int i = 0, j = 0, k = 0;
@@ -1245,7 +1245,7 @@ const uint32_t *sink_rates, int sink_len, uint32_t *supported_rates)
return k;
}
-static int rate_to_index(uint32_t find, const uint32_t *rates)
+static int rate_to_index(int find, const int *rates)
{
int i = 0;
@@ -1275,9 +1275,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int max_clock;
int bpp, mode_rate;
int link_avail, link_clock;
- uint32_t sink_rates[8];
- uint32_t supported_rates[8] = {0};
- uint32_t source_rates[8];
+ int sink_rates[8];
+ int supported_rates[8] = {0};
+ int source_rates[8];
int source_len, sink_len, supported_len;
sink_len = intel_read_sink_rates(intel_dp, sink_rates);
Since you're going through the work to redo all the link rates, does it
make more sense to have the numerical rates defined in an enum then
referenced by enumerated type in the arrays? Functionally it really
won't make any difference but it might make it easier to understand
which platforms support which rates at a glance. Certainly not a
blocking request, just a thought.
Otherwise this looks fine.
Reviewed-by: Todd Previte
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/13] drm/i915: Store the converted link rates in intel_dp->supported_rates[]
On 3/12/2015 8:10 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
No point in converting from hardware format every single time, just
store the rates in the final format under intel_dp.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 33 +++--
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1fa8cc1..d638f5e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1141,8 +1141,6 @@ static int
intel_read_sink_rates(struct intel_dp *intel_dp, int *sink_rates)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- int i = 0;
- uint16_t val;
if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
/*
@@ -1150,18 +1148,12 @@ intel_read_sink_rates(struct intel_dp *intel_dp, int
*sink_rates)
* link rate table method, so read link rates from
* supported_link_rates
*/
- for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i) {
- val = le16_to_cpu(intel_dp->supported_rates[i]);
- if (val == 0)
- break;
-
- sink_rates[i] = val * 200;
- }
+ memcpy(sink_rates, intel_dp->supported_rates,
+ sizeof(intel_dp->supported_rates));
- if (i <= 0)
- DRM_ERROR("No rates in SUPPORTED_LINK_RATES");
+ return intel_dp->num_supported_rates;
}
- return i;
+ return 0;
}
static int
@@ -3751,10 +3743,23 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
(intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
(intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1)
&&
(rev >= 0x03)) { /* eDp v1.4 or higher */
+ __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
+ int i;
+
intel_dp_dpcd_read_wake(&intel_dp->aux,
DP_SUPPORTED_LINK_RATES,
- intel_dp->supported_rates,
- sizeof(intel_dp->supported_rates));
+ supported_rates,
+ sizeof(supported_rates));
+
+ for (i = 0; i < ARRAY_SIZE(supported_rates); i++) {
+ int val = le16_to_cpu(supported_rates[i]);
+
+ if (val == 0)
+ break;
+
+ intel_dp->supported_rates[i] = val * 200;
+ }
+ intel_dp->num_supported_rates = i;
}
if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
DP_DWN_STRM_PORT_PRESENT))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c77128c..69c8437 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -627,7 +627,8 @@ struct intel_dp {
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
- __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
+ uint8_t num_supported_rates;
+ int supported_rates[DP_MAX_SUPPORTED_RATES];
struct drm_dp_aux aux;
uint8_t train_set[4];
int panel_power_up_delay;
The code looks good from here. Only thing to double check is where
intel_read_sink_rates() is called and make sure it's now expecting 0 as
a success case.
Reviewed-by: Todd Previte
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/13] drm/i915: Don't copy the DP source rates arrays
On 3/12/2015 8:10 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
The source rates don't change, so we can just point the caller at the
const arrays.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d638f5e..537f1d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1157,22 +1157,18 @@ intel_read_sink_rates(struct intel_dp *intel_dp, int
*sink_rates)
}
static int
-intel_read_source_rates(struct intel_dp *intel_dp, int *source_rates)
+intel_dp_source_rates(struct intel_dp *intel_dp, const int **source_rates)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
- int i;
- int max_default_rate;
- if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
- for (i = 0; i < ARRAY_SIZE(gen9_rates); ++i)
- source_rates[i] = gen9_rates[i];
- } else {
- /* Index of the max_link_bw supported + 1 */
- max_default_rate = (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
- for (i = 0; i < max_default_rate; ++i)
- source_rates[i] = default_rates[i];
+ if (INTEL_INFO(dev)->gen >= 9) {
+ *source_rates = gen9_rates;
+ return ARRAY_SIZE(gen9_rates);
}
- return i;
+
+ *source_rates = default_rates;
+
+ return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
}
static void
@@ -1269,12 +1265,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
int link_avail, link_clock;
int sink_rates[8];
int supported_rates[8] = {0};
- int source_rates[8];
+ const int *source_rates;
int source_len, sink_len, supported_len;
sink_len = intel_read_sink_rates(intel_dp, sink_rates);
- source_len = intel_read_source_rates(intel_dp, source_rates);
+ source_len = intel_dp_source_rates(intel_dp, &source_rates);
supported_len = intel_supported_rates(source_rates, source_len,
sink_rates, sink_len, supported_rates);
Looks like it's good to go.
Reviewed-by: Todd Previte
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix sink crc connector iteration
Regressed by this commit:
commit 3455454e18ca3f92c565700539e744c620d8276b
Author: Ander Conselvan de Oliveira
Date: Tue Mar 3 15:21:56 2015 +0200
drm/i915: Add a for_each_intel_connector macro
Cc: Ander Conselvan de Oliveira
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f7ec06..daf948e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2305,7 +2305,7 @@ static int i915_sink_crc(struct seq_file *m, void *data)
u8 crc[6];
drm_modeset_lock_all(dev);
- for_each_intel_encoder(dev, connector) {
+ for_each_intel_connector(dev, connector) {
if (connector->base.dpms != DRM_MODE_DPMS_ON)
continue;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/19] drm/i915: Remove usage of encoder->new_crtc from clock computations
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5946 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK -6 303/303 297/303 SNB 279/279 279/279 IVB 343/343 343/343 BYT 287/287 287/287 HSW -1 363/363 362/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt_drv_suspend_debugfs-reader PASS(2) DMESG_WARN(2) *ILK igt_drv_suspend_fence-restore-tiled2untiled PASS(2) DMESG_WARN(1) *ILK igt_drv_suspend_fence-restore-untiled PASS(2) DMESG_WARN(2) *ILK igt_drv_suspend_forcewake PASS(2) DMESG_WARN(2) *ILK igt_gem_workarounds_suspend-resume PASS(2) DMESG_WARN(2) *ILK igt_kms_flip_blocking-absolute-wf_vblank-interruptible PASS(2) DMESG_FAIL(2) *HSW igt_gem_pwrite_pread_snooped-copy-performance PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix vmap_batch page iterator overrun
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5947 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 303/303 303/303 SNB 279/279 279/279 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5948 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 276/276 276/276 ILK 303/303 303/303 SNB -1 279/279 278/279 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied *SNB igt_gem_persistent_relocs_forked-interruptible-faulting-reloc-thrash-inactive PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
