Re: [Intel-gfx] [PATCH] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)
Hi Tvrtko,
On Fri, 30 Oct 2015 10:22:08 +
Tvrtko Ursulin wrote:
>
> On 30/10/15 01:44, Vivek Kasireddy wrote:
> > The main goal of this subtest is to trigger the following warning in
> > the function i915_gem_object_get_fence():
> > if (WARN_ON(!obj->map_and_fenceable))
> >
> > To trigger this warning, the subtest first creates a Y-tiled object
> > and an associated framebuffer with the Y-fb modifier. Furthermore,
> > to prevent the map_and_fenceable from being set, we make sure that
> > the object does not have a normal VMA by refraining from rendering
> > to the object and by setting the rotation property upfront before
> > calling commit.
> >
> > v2: Do not call paint_squares and just use one output.
> >
> > v3: Convert an if condition to igt_require and move the plane
> > rotation requirement further up before the fb allocation.
> >
> > v4: After setting rotation to 90 and committing, change the
> > rotation to 0 and commit once more. This is to test if the i915
> > driver hits any warnings while pinning and unpinning an object that
> > has both normal and rotated views.
> >
> > v5:
> > - Add another subtest to toggle the order of rotation
> > - Exhaustively test the i915 driver's pinning and unpinning code
> > paths for any fence leaks by iterating until MAX available fences.
> >
> > Cc: Tvrtko Ursulin
> > Signed-off-by: Vivek Kasireddy
> > ---
> > tests/kms_rotation_crc.c | 84
> > 1 file changed, 84
> > insertions(+)
> >
> > diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> > index cc9847e..34f8150 100644
> > --- a/tests/kms_rotation_crc.c
> > +++ b/tests/kms_rotation_crc.c
> > @@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data,
> > enum igt_plane plane_type) igt_require_f(valid_tests, "no valid
> > crtc/connector combinations found\n"); }
> >
> > +static void test_plane_rotation_ytiled_obj(data_t *data, enum
> > igt_plane plane_type,
> > + int toggle)
> > +{
> > + igt_display_t *display = &data->display;
> > + uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
> > + uint32_t format = DRM_FORMAT_XRGB;
> > + int bpp = igt_drm_format_to_bpp(format);
> > + enum igt_commit_style commit = COMMIT_LEGACY;
> > + int fd = data->gfx_fd;
> > + igt_output_t *output = &display->outputs[0];
> > + igt_plane_t *plane;
> > + drmModeModeInfo *mode;
> > + unsigned int stride, size, w, h;
> > + uint32_t gem_handle;
> > + int num_fences = gem_available_fences(fd);
> > + int i, ret;
> > +
> > + igt_require(output != NULL && output->valid == true);
> > +
> > + plane = igt_output_get_plane(output, plane_type);
> > + igt_require(igt_plane_supports_rotation(plane));
> > +
> > + if (plane_type == IGT_PLANE_PRIMARY || plane_type ==
> > IGT_PLANE_CURSOR) {
> > + igt_require(data->display.has_universal_planes);
> > + commit = COMMIT_UNIVERSAL;
> > + }
> > +
> > + mode = igt_output_get_mode(output);
> > + w = mode->hdisplay;
> > + h = mode->vdisplay;
> > +
> > + for (stride = 512; stride < (w * bpp / 8); stride *= 2)
> > + ;
> > + for (size = 1024*1024; size < stride * h; size *= 2)
> > + ;
> > +
> > + gem_handle = gem_create(fd, size);
> > + ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y,
> > stride);
> > + igt_assert(ret == 0);
> > +
> > + do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
> > + format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
> > + &data->fb.fb_id));
> > + data->fb.width = w;
> > + data->fb.height = h;
> > + data->fb.gem_handle = gem_handle;
> > +
> > + igt_plane_set_fb(plane, NULL);
> > + igt_display_commit(display);
> > +
> > + igt_plane_set_fb(plane, &data->fb);
> > +
> > + for (i = 0; i < num_fences + 1; i++) {
> > + igt_plane_set_rotation(plane, toggle ?
> > IGT_ROTATION_0 : IGT_ROTATION_90);
> > + drmModeObjectSetProperty(fd,
> > plane->drm_plane->plane_id,
> > +DRM_MODE_OBJECT_PLANE,
> > +plane->rotation_property,
> > +plane->rotation);
> > + ret = igt_display_try_commit2(display, commit);
> > + igt_assert(ret == 0);
> > +
> > + igt_plane_set_rotation(plane, toggle ?
> > IGT_ROTATION_90 : IGT_ROTATION_0);
> > + drmModeObjectSetProperty(fd,
> > plane->drm_plane->plane_id,
> > +DRM_MODE_OBJECT_PLANE,
> > +plane->rotation_property,
> > +plane->rotation);
> > + ret = igt_display_try_commit2(display, commit);
> > + igt_assert(ret == 0);
> > + }
>
> It manages to exhaust the fences with only one object? I was
> expecting that multiple objects will be required since if it is only
> one how come it allocates more than one fence register?
Before I sent
Re: [Intel-gfx] [PATCH v7 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
On Fri, Oct 30, 2015 at 03:36:28PM -0700, Rafael Antognolli wrote: > This module is heavily based on i2c-dev. Once loaded, it provides one > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer. > > It's possible to know which connector owns this aux channel by looking > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if > the connector device pointer was correctly set in the aux helper struct. > > Two main operations are provided on the registers read and write. The > address of the register to be read or written is given using lseek. The > seek position is updated upon read or write. > > v2: > - lseek is used to select the register to read/write > - read/write are used instead of ioctl > - no blocking_notifier is used, just a direct callback > > v3: > - use drm_dp_aux_dev prefix for public functions > - chardev is named drm_dp_auxN > - read/write don't allocate a buffer anymore, and transfer up to 16 bytes a >time > - remove notifier list from the implementation > - option on menuconfig is now a boolean > - add inline stub functions to avoid breakage when this option is disabled > > v4: > - fix build system changes - actually disable this module when not selected. > > v5: > - Use kref to avoid device closing while still in use > - Don't use list, use an idr for storing aux_dev > - Remove "connector" attribute > - set aux.dev to the connector drm_connector device, instead of >drm_device > > v6: > - Use atomic_t for usage count > - Use a mutex instead of spinlock for idr lock > - Destroy chardev immediately on unregister > - other minor suggestions from Ville > > v7: > - style fixes > - error handling fixes > > Signed-off-by: Rafael Antognolli > --- > drivers/gpu/drm/Kconfig | 8 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/drm_dp_aux_dev.c | 374 > +++ > drivers/gpu/drm/drm_dp_helper.c | 5 + > include/drm/drm_dp_aux_dev.h | 50 ++ > 5 files changed, 438 insertions(+) > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c > create mode 100644 include/drm/drm_dp_aux_dev.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index c4bf9a1..daefcce 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI > bool > depends on DRM > > +config DRM_DP_AUX_CHARDEV > + bool "DRM DP AUX Interface" > + depends on DRM > + help > + Choose this option to enable a /dev/drm_dp_auxN node that allows to > + read and write values to arbitrary DPCD registers on the DP aux > + channel. > + > config DRM_KMS_HELPER > tristate > depends on DRM > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 1e9ff4c..e48ec8f 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o > drm_probe_helper.o \ > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o > > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c > b/drivers/gpu/drm/drm_dp_aux_dev.c > new file mode 100644 > index 000..1a3ca39 > --- /dev/null > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c > @@ -0,0 +1,374 @@ > +/* > + * Copyright © 2015 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: > + *Rafael Antognolli > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct drm_dp_aux_d
[Intel-gfx] [PATCH 1/1] drm/i915/dma: enforce pr_ consistency
One branch of the if clause uses pr_info, the other pr_err; change
the 'false' branch to also use pr_info. This minor oversight has gone
unfixed since the initial vga_switcheroo implementation in 6a9ee8af.
Signed-off-by: Ioan-Adrian Ratiu
---
drivers/gpu/drm/i915/i915_dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2336af9..b23f465 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -338,7 +338,7 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev,
enum vga_switcheroo_
i915_resume_switcheroo(dev);
dev->switch_power_state = DRM_SWITCH_POWER_ON;
} else {
- pr_err("switched off\n");
+ pr_info("switched off\n");
dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
i915_suspend_switcheroo(dev, pmm);
dev->switch_power_state = DRM_SWITCH_POWER_OFF;
--
2.6.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 0/2] Add drm_dp_aux chardev support.
This series implement support to a drm_dp_aux chardev that allows reading and writing an arbitrary amount of bytes to arbitrary dpcd register addresses using regular read, write and lseek operations. Rafael Antognolli (2): drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers. drm/dp: Set aux.dev to the drm_connector device, instead of drm_device. drivers/gpu/drm/Kconfig | 8 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_dp_aux_dev.c | 374 +++ drivers/gpu/drm/drm_dp_helper.c | 5 + drivers/gpu/drm/i915/intel_dp.c | 19 +- include/drm/drm_dp_aux_dev.h | 50 ++ 6 files changed, 440 insertions(+), 17 deletions(-) create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c create mode 100644 include/drm/drm_dp_aux_dev.h -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 2/2] drm/dp: Set aux.dev to the drm_connector device, instead of drm_device.
So far, the i915 driver and some other drivers set it to the drm_device,
which doesn't allow one to know which DP a given aux channel is related
to. Changing this to be the drm_connector provides proper nesting, still
allowing one to get the drm_device from it. Some drivers already set it
to the drm_connector.
This also removes the need to add a sysfs link for the i2c device under
the connector, as it will already be there.
Signed-off-by: Rafael Antognolli
---
drivers/gpu/drm/i915/intel_dp.c | 19 ++-
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8287df4..7aacc08 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1078,36 +1078,21 @@ intel_dp_aux_init(struct intel_dp *intel_dp, struct
intel_connector *connector)
intel_dp->aux_ch_ctl_reg = intel_dp->output_reg + 0x10;
intel_dp->aux.name = name;
- intel_dp->aux.dev = dev->dev;
+ intel_dp->aux.dev = connector->base.kdev;
intel_dp->aux.transfer = intel_dp_aux_transfer;
DRM_DEBUG_KMS("registering %s bus for %s\n", name,
connector->base.kdev->kobj.name);
ret = drm_dp_aux_register(&intel_dp->aux);
- if (ret < 0) {
+ if (ret < 0)
DRM_ERROR("drm_dp_aux_register() for %s failed (%d)\n",
name, ret);
- return;
- }
-
- ret = sysfs_create_link(&connector->base.kdev->kobj,
- &intel_dp->aux.ddc.dev.kobj,
- intel_dp->aux.ddc.dev.kobj.name);
- if (ret < 0) {
- DRM_ERROR("sysfs_create_link() for %s failed (%d)\n", name,
ret);
- drm_dp_aux_unregister(&intel_dp->aux);
- }
}
static void
intel_dp_connector_unregister(struct intel_connector *intel_connector)
{
- struct intel_dp *intel_dp = intel_attached_dp(&intel_connector->base);
-
- if (!intel_connector->mst_port)
- sysfs_remove_link(&intel_connector->base.kdev->kobj,
- intel_dp->aux.ddc.dev.kobj.name);
intel_connector_unregister(intel_connector);
}
--
2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
This module is heavily based on i2c-dev. Once loaded, it provides one
dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
It's possible to know which connector owns this aux channel by looking
at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
the connector device pointer was correctly set in the aux helper struct.
Two main operations are provided on the registers read and write. The
address of the register to be read or written is given using lseek. The
seek position is updated upon read or write.
v2:
- lseek is used to select the register to read/write
- read/write are used instead of ioctl
- no blocking_notifier is used, just a direct callback
v3:
- use drm_dp_aux_dev prefix for public functions
- chardev is named drm_dp_auxN
- read/write don't allocate a buffer anymore, and transfer up to 16 bytes a
time
- remove notifier list from the implementation
- option on menuconfig is now a boolean
- add inline stub functions to avoid breakage when this option is disabled
v4:
- fix build system changes - actually disable this module when not selected.
v5:
- Use kref to avoid device closing while still in use
- Don't use list, use an idr for storing aux_dev
- Remove "connector" attribute
- set aux.dev to the connector drm_connector device, instead of
drm_device
v6:
- Use atomic_t for usage count
- Use a mutex instead of spinlock for idr lock
- Destroy chardev immediately on unregister
- other minor suggestions from Ville
v7:
- style fixes
- error handling fixes
Signed-off-by: Rafael Antognolli
---
drivers/gpu/drm/Kconfig | 8 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_dp_aux_dev.c | 374 +++
drivers/gpu/drm/drm_dp_helper.c | 5 +
include/drm/drm_dp_aux_dev.h | 50 ++
5 files changed, 438 insertions(+)
create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
create mode 100644 include/drm/drm_dp_aux_dev.h
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index c4bf9a1..daefcce 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -25,6 +25,14 @@ config DRM_MIPI_DSI
bool
depends on DRM
+config DRM_DP_AUX_CHARDEV
+ bool "DRM DP AUX Interface"
+ depends on DRM
+ help
+ Choose this option to enable a /dev/drm_dp_auxN node that allows to
+ read and write values to arbitrary DPCD registers on the DP aux
+ channel.
+
config DRM_KMS_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1e9ff4c..e48ec8f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
drm_probe_helper.o \
drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
+drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
new file mode 100644
index 000..1a3ca39
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -0,0 +1,374 @@
+/*
+ * Copyright © 2015 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:
+ *Rafael Antognolli
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct drm_dp_aux_dev {
+ unsigned index;
+ struct drm_dp_aux *aux;
+ struct device *dev;
+ struct kref refcount;
+ atomic_t usecount;
+};
+
+#define DRM_AUX_MINORS 256
+#define AUX_MAX_OFFSET (1 << 20)
+static DEFINE_IDR(aux_idr);
+static DEFINE_MUTEX(aux_idr_mutex);
+static struct class *drm_dp_aux_dev_class
[Intel-gfx] [PATCH] drm/i915: Print a debug message when exceeding dotclock limit on pre-gen4
From: Ville Syrjälä
Currently there's no trace in dmesg when the gen2/3 dotclock checks
reject the modeset. Add some to avoid further head scratching.
While at it refactor the code a bit to look nicer.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 26 ++
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2b70151..1509a99 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6599,6 +6599,15 @@ static void hsw_compute_ips_config(struct intel_crtc
*crtc,
pipe_config_supports_ips(dev_priv, pipe_config);
}
+static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc)
+{
+ const struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+ /* GDG double wide on either pipe, otherwise pipe A only */
+ return INTEL_INFO(dev_priv)->gen < 4 &&
+ (crtc->pipe == PIPE_A || IS_I915G(dev_priv));
+}
+
static int intel_crtc_compute_config(struct intel_crtc *crtc,
struct intel_crtc_state *pipe_config)
{
@@ -6608,23 +6617,24 @@ static int intel_crtc_compute_config(struct intel_crtc
*crtc,
/* FIXME should check pixel clock limits on all platforms */
if (INTEL_INFO(dev)->gen < 4) {
- int clock_limit = dev_priv->max_cdclk_freq;
+ int clock_limit = dev_priv->max_cdclk_freq * 9 / 10;
/*
-* Enable pixel doubling when the dot clock
+* Enable double wide mode when the dot clock
* is > 90% of the (display) core speed.
-*
-* GDG double wide on either pipe,
-* otherwise pipe A only.
*/
- if ((crtc->pipe == PIPE_A || IS_I915G(dev)) &&
- adjusted_mode->crtc_clock > clock_limit * 9 / 10) {
+ if (intel_crtc_supports_double_wide(crtc) &&
+ adjusted_mode->crtc_clock > clock_limit) {
clock_limit *= 2;
pipe_config->double_wide = true;
}
- if (adjusted_mode->crtc_clock > clock_limit * 9 / 10)
+ if (adjusted_mode->crtc_clock > clock_limit) {
+ DRM_DEBUG_KMS("requested pixel clock (%d kHz) too high
(max: %d kHz, double wide: %s)\n",
+ adjusted_mode->crtc_clock, clock_limit,
+ yesno(pipe_config->double_wide));
return -EINVAL;
+ }
}
/*
--
2.4.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
On Fri, Oct 30, 2015 at 01:59:22PM -0700, Rafael Antognolli wrote:
> On Fri, Oct 30, 2015 at 12:04:17PM +0200, Ville Syrjälä wrote:
> > On Thu, Oct 29, 2015 at 04:23:45PM -0700, Rafael Antognolli wrote:
> > > This module is heavily based on i2c-dev. Once loaded, it provides one
> > > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
> > >
> > > It's possible to know which connector owns this aux channel by looking
> > > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
> > > the connector device pointer was correctly set in the aux helper struct.
> > >
> > > Two main operations are provided on the registers read and write. The
> > > address of the register to be read or written is given using lseek. The
> > > seek position is updated upon read or write.
> >
> > Note that we (i915 folks at least) generally like to have the changelog
> > in the commit message itself. So maybe move it here for the final
> > version.
> >
> > Anyways, this is looking rather nice now. I spotted a few remaining style
> > issues, and some error handling problems. Once those are dealt with
> > I think we should be done.
> >
> > >
> > > Signed-off-by: Rafael Antognolli
> > > ---
> > > drivers/gpu/drm/Kconfig | 8 +
> > > drivers/gpu/drm/Makefile | 1 +
> > > drivers/gpu/drm/drm_dp_aux_dev.c | 381
> > > +++
> > > drivers/gpu/drm/drm_dp_helper.c | 5 +
> > > include/drm/drm_dp_aux_dev.h | 50 +
> > > 5 files changed, 445 insertions(+)
> > > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
> > > create mode 100644 include/drm/drm_dp_aux_dev.h
> > >
> > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > > index c4bf9a1..daefcce 100644
> > > --- a/drivers/gpu/drm/Kconfig
> > > +++ b/drivers/gpu/drm/Kconfig
> > > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI
> > > bool
> > > depends on DRM
> > >
> > > +config DRM_DP_AUX_CHARDEV
> > > + bool "DRM DP AUX Interface"
> > > + depends on DRM
> > > + help
> > > + Choose this option to enable a /dev/drm_dp_auxN node that allows to
> > > + read and write values to arbitrary DPCD registers on the DP aux
> > > + channel.
> > > +
> > > config DRM_KMS_HELPER
> > > tristate
> > > depends on DRM
> > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > > index 1e9ff4c..e48ec8f 100644
> > > --- a/drivers/gpu/drm/Makefile
> > > +++ b/drivers/gpu/drm/Makefile
> > > @@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> > > drm_probe_helper.o \
> > > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> > > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> > > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> > > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> > >
> > > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> > >
> > > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> > > b/drivers/gpu/drm/drm_dp_aux_dev.c
> > > new file mode 100644
> > > index 000..16dbc2e
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> > > @@ -0,0 +1,381 @@
> > > +/*
> > > + * Copyright © 2015 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:
> > > + *Rafael Antognolli
> > > + *
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +struct drm_dp_aux_dev {
> > > + unsigned index;
> > > + struct drm_dp_aux *aux;
> > > + struct device *dev;
> > > + struct kref refcount;
> > > + atomic_t usecou
[Intel-gfx] [PATCH] Revert "drm/i915: Make prepare_plane_fb fully interruptible."
From: Ville Syrjälä
This reverts commit b26a6b35581c84124bd78b68cc02d171fbd572c9.
commit b26a6b35581c ("drm/i915: Make prepare_plane_fb fully interruptible.")
breaks GPU reset on gen3/4 machines. Go back to to non-interruptible.
Cc: Maarten Lankhorst
Cc: Ander Conselvan de Oliveira
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7687708..2b70151 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2386,10 +2386,11 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
*/
intel_runtime_pm_get(dev_priv);
+ dev_priv->mm.interruptible = false;
ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined,
pipelined_request, &view);
if (ret)
- goto err_pm;
+ goto err_interruptible;
/* Install a fence for tiled scan-out. Pre-i965 always needs a
* fence, whereas 965+ only requires a fence if using
@@ -2413,12 +2414,14 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
i915_gem_object_pin_fence(obj);
+ dev_priv->mm.interruptible = true;
intel_runtime_pm_put(dev_priv);
return 0;
err_unpin:
i915_gem_object_unpin_from_display_plane(obj, &view);
-err_pm:
+err_interruptible:
+ dev_priv->mm.interruptible = true;
intel_runtime_pm_put(dev_priv);
return ret;
}
@@ -13487,9 +13490,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
if (!obj && !old_obj)
return 0;
- ret = i915_mutex_lock_interruptible(dev);
- if (ret)
- return ret;
+ mutex_lock(&dev->struct_mutex);
if (!obj) {
ret = 0;
--
2.4.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
On Fri, Oct 30, 2015 at 12:04:17PM +0200, Ville Syrjälä wrote:
> On Thu, Oct 29, 2015 at 04:23:45PM -0700, Rafael Antognolli wrote:
> > This module is heavily based on i2c-dev. Once loaded, it provides one
> > dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
> >
> > It's possible to know which connector owns this aux channel by looking
> > at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
> > the connector device pointer was correctly set in the aux helper struct.
> >
> > Two main operations are provided on the registers read and write. The
> > address of the register to be read or written is given using lseek. The
> > seek position is updated upon read or write.
>
> Note that we (i915 folks at least) generally like to have the changelog
> in the commit message itself. So maybe move it here for the final
> version.
>
> Anyways, this is looking rather nice now. I spotted a few remaining style
> issues, and some error handling problems. Once those are dealt with
> I think we should be done.
>
> >
> > Signed-off-by: Rafael Antognolli
> > ---
> > drivers/gpu/drm/Kconfig | 8 +
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/drm_dp_aux_dev.c | 381
> > +++
> > drivers/gpu/drm/drm_dp_helper.c | 5 +
> > include/drm/drm_dp_aux_dev.h | 50 +
> > 5 files changed, 445 insertions(+)
> > create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
> > create mode 100644 include/drm/drm_dp_aux_dev.h
> >
> > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> > index c4bf9a1..daefcce 100644
> > --- a/drivers/gpu/drm/Kconfig
> > +++ b/drivers/gpu/drm/Kconfig
> > @@ -25,6 +25,14 @@ config DRM_MIPI_DSI
> > bool
> > depends on DRM
> >
> > +config DRM_DP_AUX_CHARDEV
> > + bool "DRM DP AUX Interface"
> > + depends on DRM
> > + help
> > + Choose this option to enable a /dev/drm_dp_auxN node that allows to
> > + read and write values to arbitrary DPCD registers on the DP aux
> > + channel.
> > +
> > config DRM_KMS_HELPER
> > tristate
> > depends on DRM
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 1e9ff4c..e48ec8f 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> > drm_probe_helper.o \
> > drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> > drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> > drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> > +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> >
> > obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
> >
> > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> > b/drivers/gpu/drm/drm_dp_aux_dev.c
> > new file mode 100644
> > index 000..16dbc2e
> > --- /dev/null
> > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> > @@ -0,0 +1,381 @@
> > +/*
> > + * Copyright © 2015 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:
> > + *Rafael Antognolli
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct drm_dp_aux_dev {
> > + unsigned index;
> > + struct drm_dp_aux *aux;
> > + struct device *dev;
> > + struct kref refcount;
> > + atomic_t usecount;
> > +};
> > +
> > +#define DRM_AUX_MINORS 256
> > +#define AUX_MAX_OFFSET (1 << 20)
> > +static DEFINE_IDR(aux_idr);
> > +static DEFINE_MUTEX(aux_idr_mutex);
> > +static struct class *drm_dp_aux_dev_class;
> > +static int drm_dev_major = -1;
> > +
> > +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_min
[Intel-gfx] [Regression report] Weekly regression report WW44
WW44 Regression report. This week's regressions +---+---+++ | BugId | Summary | Created on | Bisect | +---+---+++ | 92655 | [i915] LVDS screen half garbled. unable to bi | 2015-10-23 | Yes| | 92707 | [BSW] [IGT Basic] [Regression] igt / pm_backl | 2015-10-28 | No | | 92718 | [REGRESSION] 4.3.0-rc7 - Multiple identical k | 2015-10-29 | No | | 92737 | [SKL] [Regression] igt/gem_ctx_param_basic/in | 2015-10-30 | No | +---+---+++ Older regressions +---+---+++ | BugId | Summary | Created on | Bisect | +---+---+++ | 72782 | [945GM bisected] screen blank on S3 resume on | 2013-12-17 | Yes| | 81537 | [snb dp regression] dp retry forever due to s | 2014-07-19 | No | | 84855 | [ILK regression]igt kms_rotation_crc/sprite-r | 2014-10-10 | No | | 84974 | [VLV eDP-LVDS bisected] powerdomains: Screen | 2014-10-14 | Yes| | 87131 | [PNV regression] igt/gem_exec_lut_handle take | 2014-12-09 | No | | 87480 | [SNB/IVB/HSW/BYT bisected]igt/kms_force_conne | 2014-12-19 | Yes| | 87662 | [ALL 3.18 Bisected] DVI --rotation inverted c | 2014-12-24 | Yes| | 87725 | [BDW Bisected] OglBatch7 performance reduced | 2014-12-26 | Yes| | 87726 | [BDW Bisected] OglDrvCtx performance reduced | 2014-12-26 | Yes| | 88012 | [bisected BYT] complete freeze after: drm/i91 | 2015-01-04 | Yes| | 88124 | i915: regression: after DP connected monitor | 2015-01-06 | No | | 88325 | screen brightness regression on resume| 2015-01-12 | No | | 88439 | [BDW Bisected]igt/gem_reloc_vs_gpu/forked-fau | 2015-01-15 | Yes| | 89334 | [945 regression] 4.0-rc1 kernel GPU hang: ec | 2015-02-26 | No | | 89629 | [i965 regression]igt/kms_rotation_crc/sprite- | 2015-03-18 | No | | 89632 | [i965 regression]igt/kms_universal_plane/univ | 2015-03-18 | No | | 89728 | [HSW/BDW/BSW/SKL bisected] igt/pm_rps/ subcas | 2015-03-23 | Yes| | 89872 | [ HSW Bisected ] VGA was white screen when re | 2015-04-02 | Yes| | 90112 | [BSW bisected] OglGSCloth/Lightsmark/CS/ Port | 2015-04-20 | Yes| | 90134 | [BSW Bisected]GFXBench3_gl_driver/GFXBench3_g | 2015-04-22 | Yes| | 90368 | [SNB bisected igt/kms_3d has hardcoded expect | 2015-05-08 | Yes| | 90394 | [SNB Regression]igt/kms_plane/plane-position- | 2015-05-11 | No | | 90461 | [SKL Regression]boot system causes WARNING: C | 2015-05-15 | No | | 90546 | [BDW/BSW/SKL bisected]igt/pm_rpm/drm-resource | 2015-05-21 | Yes| | 90732 | [BDW/BSW Bisected]igt/gem_reloc_vs_gpu/forked | 2015-05-29 | Yes| | 90808 | [BDW Bisected]igt/gem_ctx_param_basic/invalid | 2015-06-02 | Yes| | 90994 | [BDW regression] pm_rpm subtests fail and giv | 2015-06-16 | No | | 91378 | [hsw dp regression] 06ea66b6 (5.4GHz link clo | 2015-07-17 | No | | 91592 | [pnv regression] OOPS on boot | 2015-08-09 | No | | 91844 | [HSW Regression] intel_do_flush_locked failed | 2015-09-02 | No | | 91952 | [Bisected Regression] Blank screen from boot | 2015-09-10 | Yes| | 91959 | [865g 3.19 regression] Desktop image is disto | 2015-09-10 | No | | 91974 | [bisected] unrecoverable black screen after k | 2015-09-11 | Yes| | 92050 | [regression]/bug introduced by commit [0e572f | 2015-09-19 | No | | 92083 | [regression] [git pull] drm for 4.3 | 2015-09-23 | No | | 92096 | regression/bug introduced by commit [0e572fe7 | 2015-09-24 | No | | 92097 | [regression] Linux 4.3-rc i915: WARNING: int | 2015-09-24 | No | | 92174 | PROBLEM: Intel VGA output busticated on 4.3-r | 2015-09-29 | No | | 92211 | [All Regression] [IGT Basic] igt/pm_rpm/basic | 2015-10-01 | No | | 92237 | Horrible noise (audio) via DisplayPort [regre | 2015-10-02 | Yes| | 92355 | [SKL Regression] igt/kms_fbc_crc cause DUT cr | 2015-10-09 | No | | 92414 | [Intel-gfx] As of kernel 4.3-rc1 system will | 2015-10-10 | Yes| | 92483 | [regression] Can only go to console (fbcon/fb | 2015-10-15 | No | | 92502 | [SKL] [Regression] igt/kms_flip/2x-flip-vs-ex | 2015-10-16 | No | | 92575 | [4.2 regression] Massive graphics corruption | 2015-10-21 | No | +---+---+++ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Fix failure paths around initial fbdev allocation
On Tue, Jun 30, 2015 at 10:06:27AM +0100, Lukas Wunner wrote:
> From: Tvrtko Ursulin
>
> We had two failure modes here:
>
> 1.
> Deadlock in intelfb_alloc failure path where it calls
> drm_framebuffer_remove, which grabs the struct mutex and intelfb_create
> (caller of intelfb_alloc) was already holding it.
>
> 2.
> Deadlock in intelfb_create failure path where it calls
> drm_framebuffer_unreference, which grabs the struct mutex and
> intelfb_create was already holding it.
>
> [Chris Wilson on why struct_mutex needs to be locked in the second half
> of intelfb_create: "there is a bug here where we don't take an explicit
> pin on the VMA we setup for the fbdev which requires the lock."]
The vma is pinned, the problem is that we re-lookup it a few times, which
is racy. We should instead track the vma directly, but oh well we don't.
With that clarified in the commit message this is
Reviewed-by: Daniel Vetter
>
> v2:
>* Reformat commit msg to 72 chars. (Lukas Wunner)
>* Add third failure mode. (Lukas Wunner)
>
> v5:
>* Rebase on drm-intel-nightly 2015y-09m-01d-09h-06m-08s UTC,
> rephrase commit message. (Jani Nicula)
>
> v6:
>* In intelfb_alloc, if __intel_framebuffer_create failed,
> fb will be an ERR_PTR, thus not null. So in the failure
> path we need to check for IS_ERR_OR_NULL to avoid calling
> drm_framebuffer_remove on the ERR_PTR. (Lukas Wunner)
>* Since this is init code a drm_framebuffer_unreference should
> be all we need. drm_framebuffer_remove is for framebuffers
> that userspace has created - and is getting somewhat
> defeatured. (Daniel Vetter)
>
> Signed-off-by: Tvrtko Ursulin
> Fixes: 60a5ca015ffd ("drm/i915: Add locking around
> framebuffer_references--")
> Reported-by: Lukas Wunner
> [Lukas: Create v3 + v4 + v5 + v6 based on Tvrtko's v2]
> Signed-off-by: Lukas Wunner
> Cc: Chris Wilson
> Cc: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 20
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index ec82b51..12597b5 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -119,7 +119,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> {
> struct intel_fbdev *ifbdev =
> container_of(helper, struct intel_fbdev, helper);
> - struct drm_framebuffer *fb;
> + struct drm_framebuffer *fb = NULL;
> struct drm_device *dev = helper->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct drm_mode_fb_cmd2 mode_cmd = {};
> @@ -138,6 +138,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
> sizes->surface_depth);
>
> + mutex_lock(&dev->struct_mutex);
> +
> size = mode_cmd.pitches[0] * mode_cmd.height;
> size = PAGE_ALIGN(size);
>
> @@ -165,16 +167,19 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> ret = intel_pin_and_fence_fb_obj(NULL, fb, NULL, NULL, NULL);
> if (ret) {
> DRM_ERROR("failed to pin obj: %d\n", ret);
> - goto out_fb;
> + goto out;
> }
>
> + mutex_unlock(&dev->struct_mutex);
> +
> ifbdev->fb = to_intel_framebuffer(fb);
>
> return 0;
>
> -out_fb:
> - drm_framebuffer_remove(fb);
> out:
> + mutex_unlock(&dev->struct_mutex);
> + if (!IS_ERR_OR_NULL(fb))
> + drm_framebuffer_unreference(fb);
> return ret;
> }
>
> @@ -192,8 +197,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
> int size, ret;
> bool prealloc = false;
>
> - mutex_lock(&dev->struct_mutex);
> -
> if (intel_fb &&
> (sizes->fb_width > intel_fb->base.width ||
>sizes->fb_height > intel_fb->base.height)) {
> @@ -208,7 +211,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
> ret = intelfb_alloc(helper, sizes);
> if (ret)
> - goto out_unlock;
> + return ret;
> intel_fb = ifbdev->fb;
> } else {
> DRM_DEBUG_KMS("re-using BIOS fb\n");
> @@ -220,6 +223,8 @@ static int intelfb_create(struct drm_fb_helper *helper,
> obj = intel_fb->obj;
> size = obj->base.size;
>
> + mutex_lock(&dev->struct_mutex);
> +
> info = drm_fb_helper_alloc_fbi(helper);
> if (IS_ERR(info)) {
> ret = PTR_ERR(info);
> @@ -281,7 +286,6 @@ out_destroy_fbi:
> out_unpin:
> i915_gem_object_ggtt_unpin(obj);
> drm_gem_object_unreference(&obj->base);
> -out_unlock:
> mutex_unlock(&dev->struct_mutex);
> return ret;
> }
> --
> 1.8.5.2 (Apple Git-48)
>
> __
Re: [Intel-gfx] [PATCH v6 4/4] drm/i915: Fix error handling in intelfb_create
On Sat, Oct 24, 2015 at 12:27:57AM +0200, Lukas Wunner wrote:
> intelfb_create() is called once on driver initialization. It either
> uses the fb inherited from BIOS or allocates a new one by calling
> intelfb_alloc(). Afterwards, it calls two functions which can fail:
>
> - drm_fb_helper_alloc_fbi() can fail with -ENOMEM.
> - ioremap_wc() can fail on alignment issues etc.
>
> In the case where we allocated the fb with intelfb_alloc(), if either
> of the two functions fail we currently unpin and unref the bo,
> but leak the fb. We need to unref the fb instead of the bo here.
>
> In the case where the fb was inherited from BIOS, the fb struct was
> allocated by dev_priv->display.get_initial_plane_config() and is in
> active use by a crtc, so it seems wrong to unpin and unref its bo.
> We could treat failure of the above two functions as a fatal error
> and call i915_driver_unload(), or try to limp on (fbcon won't work,
> but X11 might). Let's do the latter.
>
> However we should at least log an error message. Currently a failure
> of the above two functions is not reported at all: The negative return
> value is passed up the call stack to intel_fbdev_initial_config()
> which ignores it.
>
> To be fair, this is a corner case which few users probably ever
> experience, nevertheless we should try to get it right.
>
> Signed-off-by: Lukas Wunner
I don't think there's a leak here really. We always assign ifbdev->fb in
intelfb_alloc, which means the fbdev teardown code will take care of it.
The correct approach is probably to not unref anything at all, or if we
unref then we have to clear ifbdev->fb (since it's the reference that very
pointer is holding that we're clearing up).
-Daniel
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 12597b5..b8c11a1 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -227,6 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>
> info = drm_fb_helper_alloc_fbi(helper);
> if (IS_ERR(info)) {
> + DRM_ERROR("Failed to allocate fb_info\n");
> ret = PTR_ERR(info);
> goto out_unpin;
> }
> @@ -253,6 +254,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> ioremap_wc(dev_priv->gtt.mappable_base +
> i915_gem_obj_ggtt_offset(obj),
> size);
> if (!info->screen_base) {
> + DRM_ERROR("Failed to remap framebuffer into virtual memory\n");
> ret = -ENOSPC;
> goto out_destroy_fbi;
> }
> @@ -284,9 +286,14 @@ static int intelfb_create(struct drm_fb_helper *helper,
> out_destroy_fbi:
> drm_fb_helper_release_fbi(helper);
> out_unpin:
> - i915_gem_object_ggtt_unpin(obj);
> - drm_gem_object_unreference(&obj->base);
> - mutex_unlock(&dev->struct_mutex);
> + /* If fb was preallocated by BIOS, try to limp on. Else free it. */
> + if (prealloc)
> + mutex_unlock(&dev->struct_mutex);
> + else {
> + i915_gem_object_ggtt_unpin(obj);
> + mutex_unlock(&dev->struct_mutex);
> + drm_framebuffer_unreference(&intel_fb->base);
> + }
> return ret;
> }
>
> --
> 1.8.5.2 (Apple Git-48)
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 v6 2/4] drm/i915: Fix double unref in intelfb_alloc failure path
On Thu, Oct 22, 2015 at 01:37:18PM +0200, Lukas Wunner wrote:
> In intelfb_alloc(), if the call to intel_pin_and_fence_fb_obj() fails,
> the bo is unrefed twice: By drm_framebuffer_remove() and once more by
> drm_gem_object_unreference(). Fix it.
>
> Reported-by: Ville Syrjälä
> Signed-off-by: Lukas Wunner
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 4fd5fdf..ec82b51 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -156,8 +156,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>
> fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
> if (IS_ERR(fb)) {
> + drm_gem_object_unreference(&obj->base);
> ret = PTR_ERR(fb);
> - goto out_unref;
> + goto out;
> }
>
> /* Flush everything out, we'll be doing GTT only from now on */
> @@ -173,8 +174,6 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
>
> out_fb:
> drm_framebuffer_remove(fb);
> -out_unref:
> - drm_gem_object_unreference(&obj->base);
> out:
> return ret;
> }
> --
> 1.8.5.2 (Apple Git-48)
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [drm-intel:drm-intel-nightly 4/8] drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3143:13: error: invalid storage class for function 'dce_v10_0_is_idle'
tree: git://anongit.freedesktop.org/drm-intel drm-intel-nightly head: fba7fdd3589b453770f28caa39064b6c0141e81a commit: b8a8f412df08b100bbb6845c50f73656d677d08a [4/8] Merge remote-tracking branch 'drm-upstream/drm-next' into drm-intel-nightly config: x86_64-randconfig-x007-10252017 (attached as .config) reproduce: git checkout b8a8f412df08b100bbb6845c50f73656d677d08a # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): drivers/gpu/drm/amd/amdgpu/dce_v10_0.c: In function 'dce_v10_0_resume': >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3143:13: error: invalid storage class >> for function 'dce_v10_0_is_idle' static bool dce_v10_0_is_idle(void *handle) ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3143:1: warning: ISO C90 forbids >> mixed declarations and code [-Wdeclaration-after-statement] static bool dce_v10_0_is_idle(void *handle) ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3148:12: error: invalid storage class >> for function 'dce_v10_0_wait_for_idle' static int dce_v10_0_wait_for_idle(void *handle) ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3153:13: error: invalid storage class >> for function 'dce_v10_0_print_status' static void dce_v10_0_print_status(void *handle) ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3161:12: error: invalid storage class >> for function 'dce_v10_0_soft_reset' static int dce_v10_0_soft_reset(void *handle) ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3191:13: error: invalid storage class >> for function 'dce_v10_0_set_crtc_vblank_interrupt_state' static void dce_v10_0_set_crtc_vblank_interrupt_state(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3220:13: error: invalid storage class >> for function 'dce_v10_0_set_crtc_vline_interrupt_state' static void dce_v10_0_set_crtc_vline_interrupt_state(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3249:12: error: invalid storage class >> for function 'dce_v10_0_set_hpd_irq_state' static int dce_v10_0_set_hpd_irq_state(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3279:12: error: invalid storage class >> for function 'dce_v10_0_set_crtc_irq_state' static int dce_v10_0_set_crtc_irq_state(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3327:12: error: invalid storage class >> for function 'dce_v10_0_set_pageflip_irq_state' static int dce_v10_0_set_pageflip_irq_state(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3350:12: error: invalid storage class >> for function 'dce_v10_0_pageflip_irq' static int dce_v10_0_pageflip_irq(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3403:13: error: invalid storage class >> for function 'dce_v10_0_hpd_int_ack' static void dce_v10_0_hpd_int_ack(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3418:13: error: invalid storage class >> for function 'dce_v10_0_crtc_vblank_int_ack' static void dce_v10_0_crtc_vblank_int_ack(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3433:13: error: invalid storage class >> for function 'dce_v10_0_crtc_vline_int_ack' static void dce_v10_0_crtc_vline_int_ack(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3448:12: error: invalid storage class >> for function 'dce_v10_0_crtc_irq' static int dce_v10_0_crtc_irq(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3486:12: error: invalid storage class >> for function 'dce_v10_0_hpd_irq' static int dce_v10_0_hpd_irq(struct amdgpu_device *adev, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3511:12: error: invalid storage class >> for function 'dce_v10_0_set_clockgating_state' static int dce_v10_0_set_clockgating_state(void *handle, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3517:12: error: invalid storage class >> for function 'dce_v10_0_set_powergating_state' static int dce_v10_0_set_powergating_state(void *handle, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3541:1: error: invalid storage class >> for function 'dce_v10_0_encoder_mode_set' dce_v10_0_encoder_mode_set(struct drm_encoder *encoder, ^ >> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c:3561:13: error: invalid storage class >> for function 'dce_v10_0_encoder_prepare' static void dce_v10_0_encoder_prepare(struct drm_encoder *encoder) ^ vim +/dce_v10_0_is_idle +3143 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c aaa36a97 Alex Deucher 2015-04-20 3137 dce_v10_0_hpd_init(adev); aaa36a97 Alex Deucher 2015-04-20 3138 f6c7aba4 Michel Dänzer 2015-10-08 3139 dce_v10_0_pagef
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Share cdclk code for BDW and BXT
On Fri, Oct 30, 2015 at 05:06:27PM +0100, Daniel Vetter wrote:
> On Tue, Oct 27, 2015 at 04:31:56PM +0200, Ville Syrjälä wrote:
> > On Tue, Oct 27, 2015 at 03:43:03PM +0200, Jani Nikula wrote:
> > > On Tue, 27 Oct 2015, ville.syrj...@linux.intel.com wrote:
> > > > From: Ville Syrjälä
> > > >
> > > > The difference betwen the BXT and BDW cdclk code boils down to two
> > > > things: claming the cdclk to the maximum supported, and panel fitter
> > > > downscaling ratio
> > > >
> > > > Unifying the the max cdclk clamping is easy, just do it.
> > > >
> > > > And as far as the panel fitter is concerned, SKL+ already (ab)use the
> > > > pch pfit state for its pipe scaler information, so it will compute the
> > > > adjusted pixel rate correctly.
> > > >
> > > > So we can just use the BDW code for BXT, as long as we lift the BXT
> > > > pixel rate -> cdclk selection into the correct place.
> > >
> > > I don't like the direction taken by this patch.
> > >
> > > We have the platform specific functions to keep stuff platform
> > > specific. Now you claim bdw and bxt are almost the same, yet the
> > > functions are filled with conditionals on the platform. I value having
> > > straightforward and readable platform specific hooks much higher than
> > > the slight reduction in line count.
> >
> > I agree that it's not all that nice. The real problem is that the current
> > cdclk function pointers are just too high level. They really should
> > be at the calc_cdclk and set_cdclk level. But fixing that would require
> > actually dealing with the gmch pfit, and stuffing the power domain
> > hack (assuming we still need it) and the pfi programming for vlv/chv
> > into the lower level functions. And looks like no one has bothered to do
> > any of that.
>
> vfunc hooks imo should be high-level, with common code shared by
> extracting helpers. Trying to make vfuncs super flexible is imo an
> approach which often just leads to big trouble. We probably should rip out
> a pile of the current cdlock vfunc hooks ... Unfortunately atomic is still
> in-flight a lot, and it's also lacking some really important top-level
> vfuncs for handling modesetting.
The top level cdclk logic is (or rather should be) the same for all
platforms. Shoveling it into vfuncs doesn't make sense to me. Only
the low level details differ.
> -Daniel
>
> >
> > >
> > > BR,
> > > Jani.
> > >
> > > >
> > > > Signed-off-by: Ville Syrjälä
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 87
> > > > +---
> > > > 1 file changed, 30 insertions(+), 57 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 978f543..0c782c7 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -5932,25 +5932,6 @@ static int valleyview_calc_cdclk(struct
> > > > drm_i915_private *dev_priv,
> > > > return 20;
> > > > }
> > > >
> > > > -static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
> > > > - int max_pixclk)
> > > > -{
> > > > - /*
> > > > -* FIXME:
> > > > -* - set 19.2MHz bypass frequency if there are no active pipes
> > > > -*/
> > > > - if (max_pixclk > 576000)
> > > > - return 624000;
> > > > - else if (max_pixclk > 384000)
> > > > - return 576000;
> > > > - else if (max_pixclk > 288000)
> > > > - return 384000;
> > > > - else if (max_pixclk > 144000)
> > > > - return 288000;
> > > > - else
> > > > - return 144000;
> > > > -}
> > > > -
> > > > /* Compute the max pixel clock for new configuration. Uses atomic
> > > > state if
> > > > * that's non-NULL, look at current state otherwise. */
> > > > static int intel_mode_max_pixclk(struct drm_device *dev,
> > > > @@ -5990,21 +5971,6 @@ static int valleyview_modeset_calc_cdclk(struct
> > > > drm_atomic_state *state)
> > > > return 0;
> > > > }
> > > >
> > > > -static int broxton_modeset_calc_cdclk(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, state);
> > > > -
> > > > - if (max_pixclk < 0)
> > > > - return max_pixclk;
> > > > -
> > > > - to_intel_atomic_state(state)->cdclk =
> > > > - broxton_calc_cdclk(dev_priv, max_pixclk);
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> > > > {
> > > > unsigned int credits, default_credits;
> > > > @@ -9497,14 +9463,6 @@ void hsw_disable_pc8(struct drm_i915_private
> > > > *dev_priv)
> > > > intel_prepare_ddi(dev);
> > > > }
> > > >
> > > > -static void broxton_m
[Intel-gfx] [PATCH v2 06/14] drm/i915: Check for FIFO underruns after modeset on IVB/HSW and CPT/PPT
From: Ville Syrjälä
Due to the shared error interrupt on IVB/HSW and CPT/PPT we may not
always get an interrupt on a FIFO underrun. But we can always do an
explicit check (like we do on GMCH platforms that have no underrun
interrupt).
v2: Drop stale kerneldoc for i9xx_check_fifo_underruns() (Daniel)
Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 6 +-
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_fifo_underrun.c | 125 ++---
3 files changed, 103 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 92f8079..4066f7c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4719,9 +4719,9 @@ intel_post_enable_primary(struct drm_crtc *crtc)
if (IS_GEN2(dev))
intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
- /* Underruns don't raise interrupts, so check manually. */
- if (HAS_GMCH_DISPLAY(dev))
- i9xx_check_fifo_underruns(dev_priv);
+ /* Underruns don't always raise interrupts, so check manually. */
+ intel_check_cpu_fifo_underruns(dev_priv);
+ intel_check_pch_fifo_underruns(dev_priv);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a3bbdc..72cc272 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -959,7 +959,8 @@ void intel_cpu_fifo_underrun_irq_handler(struct
drm_i915_private *dev_priv,
enum pipe pipe);
void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
enum transcoder pch_transcoder);
-void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv);
+void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
+void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
/* i915_irq.c */
void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c
b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 54daa66..af906c7 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -84,38 +84,21 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev)
return true;
}
-/**
- * i9xx_check_fifo_underruns - check for fifo underruns
- * @dev_priv: i915 device instance
- *
- * This function checks for fifo underruns on GMCH platforms. This needs to be
- * done manually on modeset to make sure that we catch all underruns since they
- * do not generate an interrupt by themselves on these platforms.
- */
-void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv)
+static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
{
- struct intel_crtc *crtc;
-
- spin_lock_irq(&dev_priv->irq_lock);
-
- for_each_intel_crtc(dev_priv->dev, crtc) {
- u32 reg = PIPESTAT(crtc->pipe);
- u32 pipestat;
-
- if (crtc->cpu_fifo_underrun_disabled)
- continue;
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ u32 reg = PIPESTAT(crtc->pipe);
+ u32 pipestat = I915_READ(reg) & 0x;
- pipestat = I915_READ(reg) & 0x;
- if ((pipestat & PIPE_FIFO_UNDERRUN_STATUS) == 0)
- continue;
+ assert_spin_locked(&dev_priv->irq_lock);
- I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
- POSTING_READ(reg);
+ if ((pipestat & PIPE_FIFO_UNDERRUN_STATUS) == 0)
+ return;
- DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
- }
+ I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
+ POSTING_READ(reg);
- spin_unlock_irq(&dev_priv->irq_lock);
+ DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
}
static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
@@ -150,6 +133,23 @@ static void ironlake_set_fifo_underrun_reporting(struct
drm_device *dev,
ironlake_disable_display_irq(dev_priv, bit);
}
+static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc)
+{
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
+ uint32_t err_int = I915_READ(GEN7_ERR_INT);
+
+ assert_spin_locked(&dev_priv->irq_lock);
+
+ if ((err_int & ERR_INT_FIFO_UNDERRUN(pipe)) == 0)
+ return;
+
+ I915_WRITE(GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
+ POSTING_READ(GEN7_ERR_INT);
+
+ DRM_ERROR("fifo underrun on pipe %c\n", pipe_name(pipe));
+}
+
static void ivybridge_set_fifo_underrun_reporting(struct drm_device *dev,
enum pipe pipe,
[Intel-gfx] [PATCH v2 08/14] drm/i915: Disable FIFO underrun reporting around IBX transcoder B workaround
From: Ville Syrjälä
Doing the IBX transcoder B workaround causes underruns on
pipe/transcoder A. Just hide them by disabling underrun reporting for
pipe A around the workaround.
It might be possible to avoid the underruns by moving the workaround
to be applied only when enabling pipe A. But I was too lazy to try it
right now, and the current method has been proven to work, so didn't
want to change it too hastily.
Note that this can re-enable underrun reporting on pipe A if was
already disabled due to a previous actual underrun. But that's OK, we
may just get a second underrun report if another real underron occurrs
on pipe A.
v2: Note that pipe A underruns can get re-enabled due to this (Jani)
Signed-off-by: Ville Syrjälä
Reviewed-by: Jesse Barnes (v1)
---
drivers/gpu/drm/i915/intel_dp.c | 11 +++
drivers/gpu/drm/i915/intel_drv.h | 9 +
drivers/gpu/drm/i915/intel_hdmi.c | 11 +++
drivers/gpu/drm/i915/intel_sdvo.c | 11 +++
4 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1cb1f3f..3ad3405 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3957,6 +3957,13 @@ intel_dp_link_down(struct intel_dp *intel_dp)
* matching HDMI port to be enabled on transcoder A.
*/
if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) {
+ /*
+* We get CPU/PCH FIFO underruns on the other pipe when
+* doing the workaround. Sweep them under the rug.
+*/
+ intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+ intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+
/* always enable with pattern 1 (as per spec) */
DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
@@ -3966,6 +3973,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
DP &= ~DP_PORT_EN;
I915_WRITE(intel_dp->output_reg, DP);
POSTING_READ(intel_dp->output_reg);
+
+ intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
+ intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
+ intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
}
msleep(intel_dp->panel_power_down_delay);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 72cc272..35f1457 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1073,6 +1073,15 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
{
drm_wait_one_vblank(dev, pipe);
}
+static inline void
+intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe)
+{
+ const struct intel_crtc *crtc =
+ to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+
+ if (crtc->active)
+ intel_wait_for_vblank(dev, pipe);
+}
int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
struct intel_digital_port *dport,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 013bd7d..bccbe70 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1108,6 +1108,13 @@ static void intel_disable_hdmi(struct intel_encoder
*encoder)
* matching DP port to be enabled on transcoder A.
*/
if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) {
+ /*
+* We get CPU/PCH FIFO underruns on the other pipe when
+* doing the workaround. Sweep them under the rug.
+*/
+ intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+ intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
+
temp &= ~SDVO_PIPE_B_SELECT;
temp |= SDVO_ENABLE;
/*
@@ -1122,6 +1129,10 @@ static void intel_disable_hdmi(struct intel_encoder
*encoder)
temp &= ~SDVO_ENABLE;
I915_WRITE(intel_hdmi->hdmi_reg, temp);
POSTING_READ(intel_hdmi->hdmi_reg);
+
+ intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
+ intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
+ intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
}
intel_hdmi->set_infoframes(&encoder->base, false, NULL);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index c42b636..267e6cb 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1464,12 +1464,23 @@ static void intel_disable_sdvo(struct intel_encoder
*encoder)
* matching DP port to be enabled on transcoder A.
*/
if (HAS_PCH_IBX(de
[Intel-gfx] [PATCH v2 05/14] drm/i915: Re-enable PCH FIO underrun reporting after pipe has been disabled
From: Ville Syrjälä Some hardware (IVB/HSW and CPT/PPT) have a shared error interrupt for all the relevant underrun bits, so in order to keep the error interrupt enabled, we need to have underrun reporting enabled on all PCH transocders. Currently we leave the underrun reporting disabled when the pipe is off, which means we won't get any underrun interrupts when only a subset of the pipes are active. Fix the problem by re-enabling the underrun reporting after the pipe has been disabled. And to avoid the spurious underruns during pipe enable, disable the underrun reporting before embarking on the pipe enable sequence. So this way we have the error reporting disabled while running through the modeset sequence. v2: Re-enable PCH FIFO underrun reporting unconditionally on pre-HSW Signed-off-by: Ville Syrjälä Reviewed-by: Jesse Barnes (v1) --- drivers/gpu/drm/i915/intel_display.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a2b5327..92f8079 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4857,6 +4857,9 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) return; if (intel_crtc->config->has_pch_encoder) + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false); + + if (intel_crtc->config->has_pch_encoder) intel_prepare_shared_dpll(intel_crtc); if (intel_crtc->config->has_dp_encoder) @@ -4938,6 +4941,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) if (WARN_ON(intel_crtc->active)) return; + if (intel_crtc->config->has_pch_encoder) + intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, + false); + if (intel_crtc_to_shared_dpll(intel_crtc)) intel_enable_shared_dpll(intel_crtc); @@ -5085,6 +5092,8 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) ironlake_fdi_pll_disable(intel_crtc); } + + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); } static void haswell_crtc_disable(struct drm_crtc *crtc) @@ -5132,6 +5141,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder->post_disable) encoder->post_disable(encoder); + + if (intel_crtc->config->has_pch_encoder) + intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A, + true); } static void i9xx_pfit_enable(struct intel_crtc *crtc) -- 2.4.10 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
From: Ville Syrjälä We get spurious PCH FIFO underruns if we enable the reporting too soon after enabling the crtc. Move it to be the last step, after the encoder enable. Additionally we need an extra vblank wait, otherwise we still get the underruns. Presumably the pipe/fdi isn't yet fully up and running otherwise. For symmetry, disable the PCH underrun reporting as the first thing, just before encoder disable, when shutting down the crtc. v2: Do the PCH underrun enable unconditionally (Jani, Daniel) Signed-off-by: Ville Syrjälä Reviewed-by: Jesse Barnes (v1) --- 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 e4f5b1f..5ab39af 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_crtc->active = true; intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true); - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder->pre_enable) @@ -4912,6 +4911,11 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) if (HAS_PCH_CPT(dev)) cpt_verify_modeset(dev, intel_crtc->pipe); + + /* Must wait for vblank to avoid spurious PCH FIFO underruns */ + if (intel_crtc->config->has_pch_encoder) + intel_wait_for_vblank(dev, pipe); + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true); } /* IPS only exists on ULT machines and is tied to pipe A. */ @@ -5040,15 +5044,15 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; u32 reg, temp; + if (intel_crtc->config->has_pch_encoder) + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false); + for_each_encoder_on_crtc(dev, crtc, encoder) encoder->disable(encoder); drm_crtc_vblank_off(crtc); assert_vblank_disabled(crtc); - if (intel_crtc->config->has_pch_encoder) - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false); - intel_disable_pipe(intel_crtc); ironlake_pfit_disable(intel_crtc, false); -- 2.4.10 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping
On Fri, Oct 30, 2015 at 05:18:15PM +0100, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 10:17:44PM +0100, Dave Gordon wrote:
> > On 08/10/15 21:50, Wayne Boyer wrote:
> > >From: Chris Wilson
> > >
> > >A long time ago (before 3.14) we relied on a permanent pinning of the
> > >ifbdev to lock the fb in place inside the GGTT. However, the
> > >introduction of stealing the BIOS framebuffer and reusing its address in
> > >the GGTT for the fbdev has muddied waters and we use an inherited fb.
> > >However, the inherited fb is only pinned whilst it is active and we no
> > >longer have an explicit pin for the info->system_base mmapping used by
> > >the fbdev. The result is that after some aperture pressure the fbdev may
> > >be evicted, but we continue to write the fbcon into the same GGTT
> > >address - overwriting anything else that may be put into that offset.
> > >The effect is most pronounced across suspend/resume as
> > >intel_fbdev_set_suspend() does a full clear over the whole scanout.
> > >
> > >v2: rebased on latest nightly (Wayne)
> > >v3: changed i915_gem_object_ggtt_pin() to i915_gem_obj_ggtt_pin() based
> > >on Chris' review. (Wayne)
> > >
> > >Signed-off-by: Chris Wilson
> > >Cc: "Goel, Akash"
> > >Cc: Daniel Vetter
> > >Cc: Jesse Barnes
> > >Cc: sta...@vger.kernel.org
> > >Reviewed-by: Deepak S
> > >Signed-off-by: Wayne Boyer
> > >---
> > > drivers/gpu/drm/i915/intel_fbdev.c | 15 +++
> > > 1 file changed, 15 insertions(+)
> > >
> > >diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> > >b/drivers/gpu/drm/i915/intel_fbdev.c
> > >index 6532912..0ad46521 100644
> > >--- a/drivers/gpu/drm/i915/intel_fbdev.c
> > >+++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > >@@ -215,6 +215,16 @@ static int intelfb_create(struct drm_fb_helper
> > >*helper,
> > > obj = intel_fb->obj;
> > > size = obj->base.size;
> > >
> > >+ /* The fb constructor will have already pinned us (or inherited a
> > >+ * GGTT region from the BIOS) suitable for a scanout, so
> > >+ * this should just be a no-op and increment the pin count for the
> > >+ * fbdev mmapping. It does have a useful side-effect of validating
> > >+ * the pin for fbdev's use via a GGTT mmapping.
> > >+ */
> > >+ ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > >+ if (ret)
> > >+ goto out_unlock;
> > >+
> > > info = drm_fb_helper_alloc_fbi(helper);
> > > if (IS_ERR(info)) {
> > > ret = PTR_ERR(info);
> > >@@ -274,6 +284,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > > out_destroy_fbi:
> > > drm_fb_helper_release_fbi(helper);
> > > out_unpin:
> > >+ /* Once for info->screen_base mmaping... */
> > >+ i915_gem_object_ggtt_unpin(obj);
> > >+ /* ...and once for the intel_fb */
> > > i915_gem_object_ggtt_unpin(obj);
> > > drm_gem_object_unreference(&obj->base);
> > > out_unlock:
> > >@@ -514,6 +527,8 @@ static const struct drm_fb_helper_funcs
> > >intel_fb_helper_funcs = {
> > > static void intel_fbdev_destroy(struct drm_device *dev,
> > > struct intel_fbdev *ifbdev)
> > > {
> > >+ /* Release the pinning for the info->screen_base mmaping. */
> > >+ i915_gem_object_ggtt_unpin(ifbdev->fb->obj);
> > >
> > > drm_fb_helper_unregister_fbi(&ifbdev->helper);
> > > drm_fb_helper_release_fbi(&ifbdev->helper);
> >
> > Hmm .. pinning now done by i915_gem_obj_ggtt_pin(), but the unpinning
> > function is i915_gem_object_ggtt_unpin(). Just the sort of asymmetry that
> > helps everyone understand what's going on :(
> >
> > Could we not have a mass rename of the various i915_gem_obj{ect} functions
> > to ONE consistent naming convention? (Personally I prefer 'obj' because it's
> > shorter, but consistency is more important than saving just 3 letters).
>
> Of course, just needs someone to do it, and make sure to not step onto too
> many toes. I'd love if more people actually take charge of gem instead of
> piling more on top.
I have patches to remove as much of the nonsense as I could. I have sent
some of them before, but no one looked at them it seems. Now they are
about 150 patches from the top of the queue.
-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 2/2] drm/i915: Serialise updates to GGTT with access through GGTT on Braswell
On Fri, Oct 30, 2015 at 05:14:21PM +0100, Daniel Vetter wrote:
> On Fri, Oct 23, 2015 at 06:43:32PM +0100, Chris Wilson wrote:
> > When accessing through the GTT from one CPU whilst concurrently updating
> > the GGTT PTEs in another thread, the hardware likes to return random
> > data. As we have strong serialisation prevent us from modifying the PTE
> > of an active GTT mmapping, we have to conclude that it whilst modifying
> > other PTE's that error occurs. (I have not looked for any pattern such
> > as modifying PTE within the same page or cacheline as active PTE -
> > though checking whether revoking neighbouring objects should be enough
> > to test that theory.) The corruption also seems restricted to Braswell
> > and disappears with maxcpus=0. This patch stops all access through the
> > GTT by other CPUs when we update any PTE by stopping the machine around
> > the GGTT update.
> >
> > Testcase: igt/gem_concurrent_blit
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89079
> > Signed-off-by: Chris Wilson
>
> Wild guess, since it wouldn't be the first time hw engineers screwed this
> up.
>
> Cheers, Daniel
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index d1c5cf89fe77..de983c8e6e54 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2337,12 +2337,8 @@ int i915_gem_gtt_prepare_object(struct
> drm_i915_gem_object *obj)
>
> static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
> {
> -#ifdef writeq
> - writeq(pte, addr);
> -#else
> iowrite32((u32)pte, addr);
> iowrite32(pte >> 32, addr + 4);
> -#endif
Tried:
static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
{
-#ifdef writeq
- writeq(pte, addr);
-#else
- iowrite32((u32)pte, addr);
- iowrite32(pte >> 32, addr + 4);
-#endif
+ iowrite32(0, addr);
+ wmb();
+ iowrite32(upper_32_bits(pte), addr + 4);
+ iowrite32(lower_32_bits(pte), addr);
+ wmb();
}
and just the plain iowrite(lower), iowrite(upper), neither helps.
-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] drm/i915: Fall back to zero vswing/preemph if the sink doesn't like the last good values
From: Ville Syrjälä
My Lenovo STM STDP3100 miniDP->VGA dongle doesn't seem to like it when
we try to start link training with non-zero vswing/preemphasis. So when
the initial link training DPCD write fails, retry it with zero values.
Fixes a bunch of errors like so:
[drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training
Cc: Mika Kahola
Cc: Sivakumar Thulasimani
Cc: Ander Conselvan de Oliveira
Fixes: 5fa836a9d859 ("drm/i915: DP link training optimization")
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_dp.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ba4cbf5..9529a6e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3750,10 +3750,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp
*intel_dp)
DP |= DP_PORT_EN;
+again:
/* clock recovery */
if (!intel_dp_reset_link_train(intel_dp, &DP,
DP_TRAINING_PATTERN_1 |
DP_LINK_SCRAMBLING_DISABLE)) {
+ if (intel_dp->train_set_valid) {
+ DRM_DEBUG_KMS("Sink rejected link training request,
trying again with zero values\n");
+ intel_dp->train_set_valid = false;
+ goto again;
+ }
+
DRM_ERROR("failed to enable link training\n");
return;
}
--
2.4.10
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Allow unready gpu to be reset on gen8
Chris Wilson writes: > On Fri, Oct 30, 2015 at 05:18:18PM +0200, Mika Kuoppala wrote: >> Chris Wilson writes: >> >> > On Fri, Oct 30, 2015 at 04:43:49PM +0200, Mika Kuoppala wrote: >> >> Gen9 has had demonstrated cases where forcing a not ready gpu >> >> into reset has caused system hang [1]. >> >> >> >> Gen8 has never to this date demonstrated such behaviour. >> >> >> >> In our CI tests bsw sometimes ends up in a state where it claims it >> >> is not ready for reset, based on reset request, after gpu hang. >> >> >> >> Allow gen8 to reset even after claims of nonreadiness in order >> >> to keep the gpu accessible. Enhance logging so that it will be >> >> clear what conditions led to decision of proceeding or bailing out, >> >> so that we will spot if this way of forcing our will against gpu turns >> >> out to be foolhardy. >> >> >> >> References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959 >> >> Cc: Daniel Vetter >> >> Cc: Tomi Sarvela >> >> Signed-off-by: Mika Kuoppala >> >> --- >> >> drivers/gpu/drm/i915/intel_uncore.c | 9 - >> >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c >> >> b/drivers/gpu/drm/i915/intel_uncore.c >> >> index f0f97b2..47c17f2 100644 >> >> --- a/drivers/gpu/drm/i915/intel_uncore.c >> >> +++ b/drivers/gpu/drm/i915/intel_uncore.c >> >> @@ -1504,7 +1504,14 @@ not_ready: >> >> I915_WRITE(RING_RESET_CTL(engine->mmio_base), >> >> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET)); >> >> >> >> - return -EIO; >> > >> > Where's the reference for where we hit this EIO on gen8? >> > >> >> Internal CI logs, relevant part cutpasted below. If you want >> full log holler me in irc. > > So you are saying that's there no bugzilla for this... :-p Bugzilla fairy might surprise us after a good weekend rest. -Mika > -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 13/14] drm/i915: Use intel_dp->DP in eDP PLL setup
On Fri, Oct 30, 2015 at 05:00:42PM +0100, Daniel Vetter wrote:
> On Thu, Oct 29, 2015 at 09:26:02PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Use intel_dp->DP in the eDP PLL setup, instead of doing RMWs.
> >
> > To do this we need to move DP_AUDIO_OUTPUT_ENABLE setup to happen later,
> > so that we don't enable audio accidentally while configuring the PLL.
> >
> > Also intel_dp_link_down() must be made to update intel_dp->DP so that we
> > don't re-enable the port by accident when turning off the PLL. This is
> > safe now that we don't call intel_dp_link_down() during link retraining.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 41
> > ++---
> > 1 file changed, 14 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index e259803..63659e7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1548,23 +1548,18 @@ static void ironlake_set_pll_cpu_edp(struct
> > intel_dp *intel_dp)
> > struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - u32 dpa_ctl;
> >
> > DRM_DEBUG_KMS("eDP PLL enable for clock %d\n",
> > crtc->config->port_clock);
> > - dpa_ctl = I915_READ(DP_A);
> > - dpa_ctl &= ~DP_PLL_FREQ_MASK;
> >
> > - if (crtc->config->port_clock == 162000) {
> > - dpa_ctl |= DP_PLL_FREQ_162MHZ;
> > + intel_dp->DP &= ~DP_PLL_FREQ_MASK;
> > +
> > + if (crtc->config->port_clock == 162000)
> > intel_dp->DP |= DP_PLL_FREQ_162MHZ;
> > - } else {
> > - dpa_ctl |= DP_PLL_FREQ_270MHZ;
> > + else
> > intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> > - }
> > -
> > - I915_WRITE(DP_A, dpa_ctl);
> >
> > + I915_WRITE(DP_A, intel_dp->DP);
> > POSTING_READ(DP_A);
> > udelay(500);
> > }
> > @@ -1613,9 +1608,6 @@ static void intel_dp_prepare(struct intel_encoder
> > *encoder)
> > intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
> > intel_dp->DP |= DP_PORT_WIDTH(crtc->config->lane_count);
> >
> > - if (crtc->config->has_audio)
> > - intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
> > -
> > /* Split out the IBX/CPU vs CPT settings */
> >
> > if (IS_GEN7(dev) && port == PORT_A) {
> > @@ -2176,20 +2168,14 @@ static void ironlake_edp_pll_on(struct intel_dp
> > *intel_dp)
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > - u32 dpa_ctl;
> >
> > assert_pipe_disabled(dev_priv, crtc->pipe);
> > assert_dp_port_disabled(intel_dp);
> > assert_edp_pll_disabled(dev_priv);
> >
> > DRM_DEBUG_KMS("\n");
> > - dpa_ctl = I915_READ(DP_A);
> > -
> > - /* We don't adjust intel_dp->DP while tearing down the link, to
> > -* facilitate link retraining (e.g. after hotplug). Hence clear all
> > -* enable bits here to ensure that we don't enable too much. */
> > - intel_dp->DP &= ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
> > intel_dp->DP |= DP_PLL_ENABLE;
> > +
> > I915_WRITE(DP_A, intel_dp->DP);
> > POSTING_READ(DP_A);
> > udelay(200);
> > @@ -2200,19 +2186,14 @@ static void ironlake_edp_pll_off(struct intel_dp
> > *intel_dp)
> > struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > - u32 dpa_ctl;
> >
> > assert_pipe_disabled(dev_priv, crtc->pipe);
> > assert_dp_port_disabled(intel_dp);
> > assert_edp_pll_enabled(dev_priv);
> >
> > - dpa_ctl = I915_READ(DP_A);
> > + intel_dp->DP &= ~DP_PLL_ENABLE;
>
> I think we need to clear DP_AUDIO_OUTPUT_ENABLE here too, otherwise a dpms
> off->on cycle will again enable audio too early in edp_pll_on.
intel_dp_prepare() starts from scratch:
"intel_dp->DP = I915_READ(intel_dp->output_reg) & DP_DETECTED;"
>
> >
> > - /* We can't rely on the value tracked for the DP register in
> > -* intel_dp->DP because link_down must not change that (otherwise link
> > -* re-training will fail. */
> > - dpa_ctl &= ~DP_PLL_ENABLE;
> > - I915_WRITE(DP_A, dpa_ctl);
> > + I915_WRITE(DP_A, intel_dp->DP);
> > POSTING_READ(DP_A);
> > udelay(200);
> > }
> > @@ -2568,6 +2549,8 @@ static void intel_dp_enable_port(struct intel_dp
> > *intel_dp)
> > {
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *crtc =
> > + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
> >
> > /* enable with pattern 1
Re: [Intel-gfx] [PATCH] drm/i915: Pin the ifbdev for the info->system_base GGTT mmapping
On Fri, Oct 23, 2015 at 10:17:44PM +0100, Dave Gordon wrote:
> On 08/10/15 21:50, Wayne Boyer wrote:
> >From: Chris Wilson
> >
> >A long time ago (before 3.14) we relied on a permanent pinning of the
> >ifbdev to lock the fb in place inside the GGTT. However, the
> >introduction of stealing the BIOS framebuffer and reusing its address in
> >the GGTT for the fbdev has muddied waters and we use an inherited fb.
> >However, the inherited fb is only pinned whilst it is active and we no
> >longer have an explicit pin for the info->system_base mmapping used by
> >the fbdev. The result is that after some aperture pressure the fbdev may
> >be evicted, but we continue to write the fbcon into the same GGTT
> >address - overwriting anything else that may be put into that offset.
> >The effect is most pronounced across suspend/resume as
> >intel_fbdev_set_suspend() does a full clear over the whole scanout.
> >
> >v2: rebased on latest nightly (Wayne)
> >v3: changed i915_gem_object_ggtt_pin() to i915_gem_obj_ggtt_pin() based
> >on Chris' review. (Wayne)
> >
> >Signed-off-by: Chris Wilson
> >Cc: "Goel, Akash"
> >Cc: Daniel Vetter
> >Cc: Jesse Barnes
> >Cc: sta...@vger.kernel.org
> >Reviewed-by: Deepak S
> >Signed-off-by: Wayne Boyer
> >---
> > drivers/gpu/drm/i915/intel_fbdev.c | 15 +++
> > 1 file changed, 15 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> >b/drivers/gpu/drm/i915/intel_fbdev.c
> >index 6532912..0ad46521 100644
> >--- a/drivers/gpu/drm/i915/intel_fbdev.c
> >+++ b/drivers/gpu/drm/i915/intel_fbdev.c
> >@@ -215,6 +215,16 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > obj = intel_fb->obj;
> > size = obj->base.size;
> >
> >+/* The fb constructor will have already pinned us (or inherited a
> >+ * GGTT region from the BIOS) suitable for a scanout, so
> >+ * this should just be a no-op and increment the pin count for the
> >+ * fbdev mmapping. It does have a useful side-effect of validating
> >+ * the pin for fbdev's use via a GGTT mmapping.
> >+ */
> >+ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> >+if (ret)
> >+goto out_unlock;
> >+
> > info = drm_fb_helper_alloc_fbi(helper);
> > if (IS_ERR(info)) {
> > ret = PTR_ERR(info);
> >@@ -274,6 +284,9 @@ static int intelfb_create(struct drm_fb_helper *helper,
> > out_destroy_fbi:
> > drm_fb_helper_release_fbi(helper);
> > out_unpin:
> >+/* Once for info->screen_base mmaping... */
> >+i915_gem_object_ggtt_unpin(obj);
> >+/* ...and once for the intel_fb */
> > i915_gem_object_ggtt_unpin(obj);
> > drm_gem_object_unreference(&obj->base);
> > out_unlock:
> >@@ -514,6 +527,8 @@ static const struct drm_fb_helper_funcs
> >intel_fb_helper_funcs = {
> > static void intel_fbdev_destroy(struct drm_device *dev,
> > struct intel_fbdev *ifbdev)
> > {
> >+/* Release the pinning for the info->screen_base mmaping. */
> >+i915_gem_object_ggtt_unpin(ifbdev->fb->obj);
> >
> > drm_fb_helper_unregister_fbi(&ifbdev->helper);
> > drm_fb_helper_release_fbi(&ifbdev->helper);
>
> Hmm .. pinning now done by i915_gem_obj_ggtt_pin(), but the unpinning
> function is i915_gem_object_ggtt_unpin(). Just the sort of asymmetry that
> helps everyone understand what's going on :(
>
> Could we not have a mass rename of the various i915_gem_obj{ect} functions
> to ONE consistent naming convention? (Personally I prefer 'obj' because it's
> shorter, but consistency is more important than saving just 3 letters).
Of course, just needs someone to do it, and make sure to not step onto too
many toes. I'd love if more people actually take charge of gem instead of
piling more on top.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
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 2/2] drm/i915: Serialise updates to GGTT with access through GGTT on Braswell
On Fri, Oct 23, 2015 at 06:43:32PM +0100, Chris Wilson wrote:
> When accessing through the GTT from one CPU whilst concurrently updating
> the GGTT PTEs in another thread, the hardware likes to return random
> data. As we have strong serialisation prevent us from modifying the PTE
> of an active GTT mmapping, we have to conclude that it whilst modifying
> other PTE's that error occurs. (I have not looked for any pattern such
> as modifying PTE within the same page or cacheline as active PTE -
> though checking whether revoking neighbouring objects should be enough
> to test that theory.) The corruption also seems restricted to Braswell
> and disappears with maxcpus=0. This patch stops all access through the
> GTT by other CPUs when we update any PTE by stopping the machine around
> the GGTT update.
>
> Testcase: igt/gem_concurrent_blit
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89079
> Signed-off-by: Chris Wilson
Wild guess, since it wouldn't be the first time hw engineers screwed this
up.
Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index d1c5cf89fe77..de983c8e6e54 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2337,12 +2337,8 @@ int i915_gem_gtt_prepare_object(struct
drm_i915_gem_object *obj)
static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
{
-#ifdef writeq
- writeq(pte, addr);
-#else
iowrite32((u32)pte, addr);
iowrite32(pte >> 32, addr + 4);
-#endif
}
static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
--
Daniel Vetter
Software Engineer, Intel Corporation
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: make A0 wa's applied to A1
On Mon, Oct 26, 2015 at 10:48:58AM +, tim.g...@intel.com wrote:
> From: Tim Gore
>
> Since A1 chips use the same GPU as A0, they need all the
> same wa's in the i915 driver. Update some conditionals
> to do this.
Neither summary nor commit message mentions that this is for bxt. Please
fix.
-Daniel
>
> Signed-off-by: Tim Gore
> ---
> drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
> drivers/gpu/drm/i915/intel_lrc.c| 8
> drivers/gpu/drm/i915/intel_pm.c | 2 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
> b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c0281df..6ec7b23 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -309,7 +309,7 @@ static int guc_ucode_xfer(struct drm_i915_private
> *dev_priv)
>
> /* WaDisableMinuteIaClockGating:skl,bxt */
> if (IS_SKL_REVID(dev, 0, SKL_REVID_B0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) {
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
> I915_WRITE(GUC_SHIM_CONTROL, (I915_READ(GUC_SHIM_CONTROL) &
> ~GUC_ENABLE_MIA_CLOCK_GATING));
> }
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 3265427..971d3f2 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -285,7 +285,7 @@ static bool disable_lite_restore_wa(struct
> intel_engine_cs *ring)
> struct drm_device *dev = ring->dev;
>
> return (IS_SKL_REVID(dev, 0, SKL_REVID_B0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) &&
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) &&
> (ring->id == VCS || ring->id == VCS2);
> }
>
> @@ -1315,7 +1315,7 @@ static int gen9_init_indirectctx_bb(struct
> intel_engine_cs *ring,
>
> /* WaDisableCtxRestoreArbitration:skl,bxt */
> if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0))
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1))
> wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_DISABLE);
>
> /* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
> @@ -1341,7 +1341,7 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
> *ring,
>
> /* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
> if (IS_SKL_REVID(dev, 0, SKL_REVID_B0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) {
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
> wa_ctx_emit(batch, index, MI_LOAD_REGISTER_IMM(1));
> wa_ctx_emit(batch, index, GEN9_SLICE_COMMON_ECO_CHICKEN0);
> wa_ctx_emit(batch, index,
> @@ -1351,7 +1351,7 @@ static int gen9_init_perctx_bb(struct intel_engine_cs
> *ring,
>
> /* WaDisableCtxRestoreArbitration:skl,bxt */
> if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0))
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1))
> wa_ctx_emit(batch, index, MI_ARB_ON_OFF | MI_ARB_ENABLE);
>
> wa_ctx_emit(batch, index, MI_BATCH_BUFFER_END);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0fb0459..911f837 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4691,7 +4691,7 @@ static void gen9_enable_rc6(struct drm_device *dev)
> "on" : "off");
> /* WaRsUseTimeoutMode */
> if (IS_SKL_REVID(dev, 0, SKL_REVID_D0) ||
> - IS_BXT_REVID(dev, 0, BXT_REVID_A0)) {
> + IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
> I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us */
> I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> GEN7_RC_CTL_TO_MODE |
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 02a5bb0..368d78b 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1099,11 +1099,11 @@ static int bxt_init_workarounds(struct
> intel_engine_cs *ring)
>
> /* WaStoreMultiplePTEenable:bxt */
> /* This is a requirement according to Hardware specification */
> - if (IS_BXT_REVID(dev, 0, BXT_REVID_A0))
> + if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
> I915_WRITE(TILECTL, I915_READ(TILECTL) | TILECTL_TLBPF);
>
> /* WaSetClckGatingDisableMedia:bxt */
> - if (IS_BXT_REVID(dev, 0, BXT_REVID_A0)) {
> + if (IS_BXT_REVID(dev, 0, BXT_REVID_A1)) {
> I915_WRITE(GEN7_MISCCPCTL, (I915_READ(GEN7_MISCCPCTL) &
> ~GEN8_DOP_CLOCK_GATE_MEDIA_ENABLE));
> }
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: add quirk to enable backlight on Dell Chromebook 11 (2015)
On 10/30/2015 05:50 AM, Jani Nikula wrote:
Reported-by: Keith Webb
Suggested-by: Keith Webb
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106671
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cfce7ea89d08..ec159df836a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14809,6 +14809,9 @@ static struct intel_quirk intel_quirks[] = {
/* Dell Chromebook 11 */
{ 0x0a06, 0x1028, 0x0a35, quirk_backlight_present },
+
+ /* Dell Chromebook 11 (2015 version) */
+ { 0x0a16, 0x1028, 0x0a35, quirk_backlight_present },
};
static void intel_init_quirks(struct drm_device *dev)
Reviewed-by: Clint Taylor
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915/gen9: Check BIOS RC6 setup before enabling RC6
On Fri, Oct 30, 2015 at 05:00:49PM +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> configuration registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
>
> v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
> not enabled in case BIOS disabled RC6.
>
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble
> ---
> drivers/gpu/drm/i915/intel_pm.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2c7c9fc..1ec52f2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -30,6 +30,7 @@
> #include "intel_drv.h"
> #include "../../../platform/x86/intel_ips.h"
> #include
> +#include
>
> /**
> * RC6 is a special power stage which allows the GPU to enter an very
> @@ -4763,6 +4764,20 @@ static void gen9_enable_rc6(struct drm_device *dev)
> struct intel_engine_cs *ring;
> uint32_t rc6_mask = 0;
> int unused;
> + bool hw_rc6_enabled, sw_rc6_enabled;
> + struct device *device = &dev->pdev->dev;
> +
> + /* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
> + hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> + (GEN6_RC_CTL_RC6_ENABLE |
> GEN6_RC_CTL_HW_ENABLE);
> + sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
> + && (I915_READ(GEN6_RC_STATE) & 0x4);
> + if (!(hw_rc6_enabled || sw_rc6_enabled)) {
> + i915.enable_rc6 = 0;
> + pm_runtime_forbid(device);
> + DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");
One more: You don't disable runtime PM here, only RC6. Please fix this.
-Daniel
> + }
> +
>
> /* 1a: Software RC state - RC0 */
> I915_WRITE(GEN6_RC_STATE, 0);
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 v2 1/1] drm/i915/gen9: Check BIOS RC6 setup before enabling RC6
On Fri, Oct 30, 2015 at 05:00:49PM +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> configuration registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
>
> v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
> not enabled in case BIOS disabled RC6.
>
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble
> ---
> drivers/gpu/drm/i915/intel_pm.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2c7c9fc..1ec52f2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -30,6 +30,7 @@
> #include "intel_drv.h"
> #include "../../../platform/x86/intel_ips.h"
> #include
> +#include
>
> /**
> * RC6 is a special power stage which allows the GPU to enter an very
> @@ -4763,6 +4764,20 @@ static void gen9_enable_rc6(struct drm_device *dev)
> struct intel_engine_cs *ring;
> uint32_t rc6_mask = 0;
> int unused;
> + bool hw_rc6_enabled, sw_rc6_enabled;
> + struct device *device = &dev->pdev->dev;
> +
> + /* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
> + hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> + (GEN6_RC_CTL_RC6_ENABLE |
> GEN6_RC_CTL_HW_ENABLE);
> + sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
> + && (I915_READ(GEN6_RC_STATE) & 0x4);
Please add a define for the magic value 0x4. Also, should we check
this on earlier platforms too? In that case a small helper and calling it
everywhere would be great.
-Daniel
> + if (!(hw_rc6_enabled || sw_rc6_enabled)) {
> + i915.enable_rc6 = 0;
> + pm_runtime_forbid(device);
> + DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");
> + }
> +
>
> /* 1a: Software RC state - RC0 */
> I915_WRITE(GEN6_RC_STATE, 0);
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 3/3] drm/i915: Share cdclk code for BDW and BXT
On Tue, Oct 27, 2015 at 04:31:56PM +0200, Ville Syrjälä wrote:
> On Tue, Oct 27, 2015 at 03:43:03PM +0200, Jani Nikula wrote:
> > On Tue, 27 Oct 2015, ville.syrj...@linux.intel.com wrote:
> > > From: Ville Syrjälä
> > >
> > > The difference betwen the BXT and BDW cdclk code boils down to two
> > > things: claming the cdclk to the maximum supported, and panel fitter
> > > downscaling ratio
> > >
> > > Unifying the the max cdclk clamping is easy, just do it.
> > >
> > > And as far as the panel fitter is concerned, SKL+ already (ab)use the
> > > pch pfit state for its pipe scaler information, so it will compute the
> > > adjusted pixel rate correctly.
> > >
> > > So we can just use the BDW code for BXT, as long as we lift the BXT
> > > pixel rate -> cdclk selection into the correct place.
> >
> > I don't like the direction taken by this patch.
> >
> > We have the platform specific functions to keep stuff platform
> > specific. Now you claim bdw and bxt are almost the same, yet the
> > functions are filled with conditionals on the platform. I value having
> > straightforward and readable platform specific hooks much higher than
> > the slight reduction in line count.
>
> I agree that it's not all that nice. The real problem is that the current
> cdclk function pointers are just too high level. They really should
> be at the calc_cdclk and set_cdclk level. But fixing that would require
> actually dealing with the gmch pfit, and stuffing the power domain
> hack (assuming we still need it) and the pfi programming for vlv/chv
> into the lower level functions. And looks like no one has bothered to do
> any of that.
vfunc hooks imo should be high-level, with common code shared by
extracting helpers. Trying to make vfuncs super flexible is imo an
approach which often just leads to big trouble. We probably should rip out
a pile of the current cdlock vfunc hooks ... Unfortunately atomic is still
in-flight a lot, and it's also lacking some really important top-level
vfuncs for handling modesetting.
-Daniel
>
> >
> > BR,
> > Jani.
> >
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 87
> > > +---
> > > 1 file changed, 30 insertions(+), 57 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 978f543..0c782c7 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -5932,25 +5932,6 @@ static int valleyview_calc_cdclk(struct
> > > drm_i915_private *dev_priv,
> > > return 20;
> > > }
> > >
> > > -static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
> > > - int max_pixclk)
> > > -{
> > > - /*
> > > - * FIXME:
> > > - * - set 19.2MHz bypass frequency if there are no active pipes
> > > - */
> > > - if (max_pixclk > 576000)
> > > - return 624000;
> > > - else if (max_pixclk > 384000)
> > > - return 576000;
> > > - else if (max_pixclk > 288000)
> > > - return 384000;
> > > - else if (max_pixclk > 144000)
> > > - return 288000;
> > > - else
> > > - return 144000;
> > > -}
> > > -
> > > /* Compute the max pixel clock for new configuration. Uses atomic state
> > > if
> > > * that's non-NULL, look at current state otherwise. */
> > > static int intel_mode_max_pixclk(struct drm_device *dev,
> > > @@ -5990,21 +5971,6 @@ static int valleyview_modeset_calc_cdclk(struct
> > > drm_atomic_state *state)
> > > return 0;
> > > }
> > >
> > > -static int broxton_modeset_calc_cdclk(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, state);
> > > -
> > > - if (max_pixclk < 0)
> > > - return max_pixclk;
> > > -
> > > - to_intel_atomic_state(state)->cdclk =
> > > - broxton_calc_cdclk(dev_priv, max_pixclk);
> > > -
> > > - return 0;
> > > -}
> > > -
> > > static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> > > {
> > > unsigned int credits, default_credits;
> > > @@ -9497,14 +9463,6 @@ void hsw_disable_pc8(struct drm_i915_private
> > > *dev_priv)
> > > intel_prepare_ddi(dev);
> > > }
> > >
> > > -static void broxton_modeset_commit_cdclk(struct drm_atomic_state
> > > *old_state)
> > > -{
> > > - struct drm_device *dev = old_state->dev;
> > > - unsigned int req_cdclk = to_intel_atomic_state(old_state)->cdclk;
> > > -
> > > - broxton_set_cdclk(dev, req_cdclk);
> > > -}
> > > -
> > > /* compute the max rate for new configuration */
> > > static int ilk_max_pixel_rate(struct drm_atomic_state *state)
> > > {
> > > @@ -9621,14 +9579,31 @@ static int broadwell_modeset_calc_cdclk(struct
> > > drm_atomic_state *state)
> > >* FIXME should also account for plane ratio
> > >* once 64bpp pixel formats are supported.
> > >*/
>
Re: [Intel-gfx] [PATCH 14/14] drm/i915: Configure eDP PLL freq from ironlake_edp_pll_on()
On Thu, Oct 29, 2015 at 09:26:03PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> ironlake_set_pll_cpu_edp() only gets called just before
> ironlake_edp_pll_on(), so just pull the code into ironlake_edp_pll_on().
>
> Also toss in a debug print into ironlake_edp_pll_off() to match the one
> we have in ironlake_edp_pll_on().
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_dp.c | 45
> +
> 1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 63659e7..ba4cbf5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1542,28 +1542,6 @@ found:
> return true;
> }
>
> -static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp)
> -{
> - struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> - struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - DRM_DEBUG_KMS("eDP PLL enable for clock %d\n",
> - crtc->config->port_clock);
> -
> - intel_dp->DP &= ~DP_PLL_FREQ_MASK;
> -
> - if (crtc->config->port_clock == 162000)
> - intel_dp->DP |= DP_PLL_FREQ_162MHZ;
> - else
> - intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> -
> - I915_WRITE(DP_A, intel_dp->DP);
> - POSTING_READ(DP_A);
> - udelay(500);
> -}
> -
> void intel_dp_set_link_params(struct intel_dp *intel_dp,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -2173,7 +2151,20 @@ static void ironlake_edp_pll_on(struct intel_dp
> *intel_dp)
> assert_dp_port_disabled(intel_dp);
> assert_edp_pll_disabled(dev_priv);
>
> - DRM_DEBUG_KMS("\n");
> + DRM_DEBUG_KMS("enabling eDP PLL for clock %d\n",
> + crtc->config->port_clock);
> +
> + intel_dp->DP &= ~DP_PLL_FREQ_MASK;
> +
> + if (crtc->config->port_clock == 162000)
> + intel_dp->DP |= DP_PLL_FREQ_162MHZ;
> + else
> + intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> +
> + I915_WRITE(DP_A, intel_dp->DP);
> + POSTING_READ(DP_A);
> + udelay(500);
> +
> intel_dp->DP |= DP_PLL_ENABLE;
>
> I915_WRITE(DP_A, intel_dp->DP);
> @@ -2191,6 +2182,8 @@ static void ironlake_edp_pll_off(struct intel_dp
> *intel_dp)
> assert_dp_port_disabled(intel_dp);
> assert_edp_pll_enabled(dev_priv);
>
> + DRM_DEBUG_KMS("disabling eDP PLL\n");
> +
> intel_dp->DP &= ~DP_PLL_ENABLE;
>
> I915_WRITE(DP_A, intel_dp->DP);
> @@ -2390,6 +2383,8 @@ static void ilk_post_disable_dp(struct intel_encoder
> *encoder)
> enum port port = dp_to_dig_port(intel_dp)->port;
>
> intel_dp_link_down(intel_dp);
> +
> + /* Only ilk+ has port A */
> if (port == PORT_A)
> ironlake_edp_pll_off(intel_dp);
> }
> @@ -2670,10 +2665,8 @@ static void g4x_pre_enable_dp(struct intel_encoder
> *encoder)
> }
>
> /* Only ilk+ has port A */
> - if (port == PORT_A) {
> - ironlake_set_pll_cpu_edp(intel_dp);
> + if (port == PORT_A)
> ironlake_edp_pll_on(intel_dp);
> - }
> }
>
> static void vlv_detach_power_sequencer(struct intel_dp *intel_dp)
> --
> 2.4.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 13/14] drm/i915: Use intel_dp->DP in eDP PLL setup
On Thu, Oct 29, 2015 at 09:26:02PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Use intel_dp->DP in the eDP PLL setup, instead of doing RMWs.
>
> To do this we need to move DP_AUDIO_OUTPUT_ENABLE setup to happen later,
> so that we don't enable audio accidentally while configuring the PLL.
>
> Also intel_dp_link_down() must be made to update intel_dp->DP so that we
> don't re-enable the port by accident when turning off the PLL. This is
> safe now that we don't call intel_dp_link_down() during link retraining.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 41
> ++---
> 1 file changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e259803..63659e7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1548,23 +1548,18 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp
> *intel_dp)
> struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> struct drm_device *dev = crtc->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 dpa_ctl;
>
> DRM_DEBUG_KMS("eDP PLL enable for clock %d\n",
> crtc->config->port_clock);
> - dpa_ctl = I915_READ(DP_A);
> - dpa_ctl &= ~DP_PLL_FREQ_MASK;
>
> - if (crtc->config->port_clock == 162000) {
> - dpa_ctl |= DP_PLL_FREQ_162MHZ;
> + intel_dp->DP &= ~DP_PLL_FREQ_MASK;
> +
> + if (crtc->config->port_clock == 162000)
> intel_dp->DP |= DP_PLL_FREQ_162MHZ;
> - } else {
> - dpa_ctl |= DP_PLL_FREQ_270MHZ;
> + else
> intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> - }
> -
> - I915_WRITE(DP_A, dpa_ctl);
>
> + I915_WRITE(DP_A, intel_dp->DP);
> POSTING_READ(DP_A);
> udelay(500);
> }
> @@ -1613,9 +1608,6 @@ static void intel_dp_prepare(struct intel_encoder
> *encoder)
> intel_dp->DP |= DP_VOLTAGE_0_4 | DP_PRE_EMPHASIS_0;
> intel_dp->DP |= DP_PORT_WIDTH(crtc->config->lane_count);
>
> - if (crtc->config->has_audio)
> - intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
> -
> /* Split out the IBX/CPU vs CPT settings */
>
> if (IS_GEN7(dev) && port == PORT_A) {
> @@ -2176,20 +2168,14 @@ static void ironlake_edp_pll_on(struct intel_dp
> *intel_dp)
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - u32 dpa_ctl;
>
> assert_pipe_disabled(dev_priv, crtc->pipe);
> assert_dp_port_disabled(intel_dp);
> assert_edp_pll_disabled(dev_priv);
>
> DRM_DEBUG_KMS("\n");
> - dpa_ctl = I915_READ(DP_A);
> -
> - /* We don't adjust intel_dp->DP while tearing down the link, to
> - * facilitate link retraining (e.g. after hotplug). Hence clear all
> - * enable bits here to ensure that we don't enable too much. */
> - intel_dp->DP &= ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
> intel_dp->DP |= DP_PLL_ENABLE;
> +
> I915_WRITE(DP_A, intel_dp->DP);
> POSTING_READ(DP_A);
> udelay(200);
> @@ -2200,19 +2186,14 @@ static void ironlake_edp_pll_off(struct intel_dp
> *intel_dp)
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> - u32 dpa_ctl;
>
> assert_pipe_disabled(dev_priv, crtc->pipe);
> assert_dp_port_disabled(intel_dp);
> assert_edp_pll_enabled(dev_priv);
>
> - dpa_ctl = I915_READ(DP_A);
> + intel_dp->DP &= ~DP_PLL_ENABLE;
I think we need to clear DP_AUDIO_OUTPUT_ENABLE here too, otherwise a dpms
off->on cycle will again enable audio too early in edp_pll_on.
>
> - /* We can't rely on the value tracked for the DP register in
> - * intel_dp->DP because link_down must not change that (otherwise link
> - * re-training will fail. */
> - dpa_ctl &= ~DP_PLL_ENABLE;
> - I915_WRITE(DP_A, dpa_ctl);
> + I915_WRITE(DP_A, intel_dp->DP);
> POSTING_READ(DP_A);
> udelay(200);
> }
> @@ -2568,6 +2549,8 @@ static void intel_dp_enable_port(struct intel_dp
> *intel_dp)
> {
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
> struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *crtc =
> + to_intel_crtc(dp_to_dig_port(intel_dp)->base.base.crtc);
>
> /* enable with pattern 1 (as per spec) */
> _intel_dp_set_link_train(intel_dp, &intel_dp->DP,
> @@ -2583,6 +2566,8 @@ static void intel_dp_enable_port(struct intel_dp
> *intel_dp)
>* fail when the power sequencer is freshly used for this port.
>*/
> intel_dp->DP |= DP_PORT_EN;
>
[Intel-gfx] [PATCH] tests/drv_hangman: Fix uninitialized tail usage
Tail needs to be in outer scope as it is used after loop
continuation destroying its scope.
Reported-by: Thomas Wood
Cc: Thomas Wood
Signed-off-by: Mika Kuoppala
---
tests/drv_hangman.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/drv_hangman.c b/tests/drv_hangman.c
index 6caf910..280f903 100644
--- a/tests/drv_hangman.c
+++ b/tests/drv_hangman.c
@@ -240,6 +240,7 @@ static void check_error_state(const int gen,
size_t line_size = 0;
char *ring_name = NULL;
bool bb_ok = false, req_ok = false, ringbuf_ok = false;
+ uint32_t tail;
debug_fd = igt_debugfs_open("i915_error_state", O_RDONLY);
igt_assert_lte(0, debug_fd);
@@ -252,7 +253,6 @@ static void check_error_state(const int gen,
uint64_t gtt_offset;
int req_matched = 0;
int requests;
- uint32_t tail;
int ringbuf_matched = 0;
int i, items;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/7] drm/i915: Add csr programming registers to dmc debugfs entry
We check these to determine firmware loading status. Include
them to help to debug causes of firmware loading fails.
v2: Move all CSR specific registers to i915_reg.h (Ville)
v3: Rebase
v4: Rebase (RPM ref)
Signed-off-by: Mika Kuoppala
Reviewed-by: Imre Deak
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 ---
drivers/gpu/drm/i915/i915_reg.h | 10 ++
drivers/gpu/drm/i915/intel_csr.c| 13 -
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2c33770..f6468be 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2801,17 +2801,17 @@ static int i915_dmc_info(struct seq_file *m, void
*unused)
csr = &dev_priv->csr;
+ intel_runtime_pm_get(dev_priv);
+
seq_printf(m, "fw loaded: %s\n", yesno(csr->dmc_payload != NULL));
seq_printf(m, "path: %s\n", csr->fw_path);
if (!csr->dmc_payload)
- return 0;
+ goto out;
seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
CSR_VERSION_MINOR(csr->version));
- intel_runtime_pm_get(dev_priv);
-
if (IS_SKYLAKE(dev) && csr->version >= CSR_VERSION(1, 6)) {
seq_printf(m, "DC3 -> DC5 count: %d\n",
I915_READ(SKL_CSR_DC3_DC5_COUNT));
@@ -2822,6 +2822,11 @@ static int i915_dmc_info(struct seq_file *m, void
*unused)
I915_READ(BXT_CSR_DC3_DC5_COUNT));
}
+out:
+ seq_printf(m, "program base: 0x%08x\n", I915_READ(CSR_PROGRAM(0)));
+ seq_printf(m, "ssp base: 0x%08x\n", I915_READ(CSR_SSP_BASE));
+ seq_printf(m, "htp: 0x%08x\n", I915_READ(CSR_HTP_SKL));
+
intel_runtime_pm_put(dev_priv);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c563ead..72bbed2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5697,6 +5697,16 @@ enum skl_disp_power_wells {
#define GAMMA_MODE_MODE_SPLIT (3 << 0)
/* DMC/CSR */
+#define CSR_PROGRAM(i) (0x8 + (i) * 4)
+#define CSR_SSP_BASE_ADDR_GEN9 0x2FC0
+#define CSR_HTP_ADDR_SKL 0x00500034
+#define CSR_SSP_BASE 0x8F074
+#define CSR_HTP_SKL0x8F004
+#define CSR_LAST_WRITE 0x8F034
+#define CSR_LAST_WRITE_VALUE 0xc003b400
+/* MMIO address range for CSR program (0x8 - 0x82FFF) */
+#define CSR_MMIO_START_RANGE 0x8
+#define CSR_MMIO_END_RANGE 0x8
#define SKL_CSR_DC3_DC5_COUNT 0x80030
#define SKL_CSR_DC5_DC6_COUNT 0x8002C
#define BXT_CSR_DC3_DC5_COUNT 0x80038
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 25b6ba7..7dc5390 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -49,21 +49,8 @@ MODULE_FIRMWARE(I915_CSR_BXT);
#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23)
-/*
-* SKL CSR registers for DC5 and DC6
-*/
-#define CSR_PROGRAM(i) (0x8 + (i) * 4)
-#define CSR_SSP_BASE_ADDR_GEN9 0x2FC0
-#define CSR_HTP_ADDR_SKL 0x00500034
-#define CSR_SSP_BASE 0x8F074
-#define CSR_HTP_SKL0x8F004
-#define CSR_LAST_WRITE 0x8F034
-#define CSR_LAST_WRITE_VALUE 0xc003b400
-/* MMIO address range for CSR program (0x8 - 0x82FFF) */
#define CSR_MAX_FW_SIZE0x2FFF
#define CSR_DEFAULT_FW_OFFSET 0x
-#define CSR_MMIO_START_RANGE 0x8
-#define CSR_MMIO_END_RANGE 0x8
struct intel_css_header {
/* 0x09 for DMC */
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/i915/skl: Expose DC5/DC6 entry counts
From: Damien Lespiau
The CSR firmware expose two counters, handy to check if we are indeed
entering DC5/DC6.
v2: Rebase
v3: Take RPM ref before reading (Imre)
Signed-off-by: Damien Lespiau
Reviewed-by: Rodrigo Vivi (v1)
Signed-off-by: Mika Kuoppala
Reviewed-by: Imre Deak
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 +++
drivers/gpu/drm/i915/i915_reg.h | 4
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 4ed2797..c30580b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2810,6 +2810,17 @@ static int i915_dmc_info(struct seq_file *m, void
*unused)
seq_printf(m, "version: %d.%d\n", CSR_VERSION_MAJOR(csr->version),
CSR_VERSION_MINOR(csr->version));
+ intel_runtime_pm_get(dev_priv);
+
+ if (IS_SKYLAKE(dev) && csr->version >= CSR_VERSION(1, 6)) {
+ seq_printf(m, "DC3 -> DC5 count: %d\n",
+ I915_READ(SKL_CSR_DC3_DC5_COUNT));
+ seq_printf(m, "DC5 -> DC6 count: %d\n",
+ I915_READ(SKL_CSR_DC5_DC6_COUNT));
+ }
+
+ intel_runtime_pm_put(dev_priv);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8942532..bf9bddd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5696,6 +5696,10 @@ enum skl_disp_power_wells {
#define GAMMA_MODE_MODE_12BIT (2 << 0)
#define GAMMA_MODE_MODE_SPLIT (3 << 0)
+/* DMC/CSR */
+#define SKL_CSR_DC3_DC5_COUNT 0x80030
+#define SKL_CSR_DC5_DC6_COUNT 0x8002C
+
/* interrupts */
#define DE_MASTER_IRQ_CONTROL (1 << 31)
#define DE_SPRITEB_FLIP_DONE(1 << 29)
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Clean up eDP PLL state asserts
On Thu, Oct 29, 2015 at 09:26:01PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Rewrite the eDP PLL state asserts to conform to our usual state assert
> style.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_dp.c | 54
> +
> 1 file changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 763b0ef..e259803 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2142,21 +2142,48 @@ static void intel_edp_backlight_power(struct
> intel_connector *connector,
> _intel_edp_backlight_off(intel_dp);
> }
>
> +static const char *state_string(bool enabled)
> +{
> + return enabled ? "on" : "off";
> +}
> +
> +static void assert_dp_port(struct intel_dp *intel_dp, bool state)
> +{
> + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev);
> + bool cur_state = I915_READ(intel_dp->output_reg) & DP_PORT_EN;
> +
> + I915_STATE_WARN(cur_state != state,
> + "DP port %c state assertion failure (expected %s,
> current %s)\n",
> + port_name(dig_port->port),
> + state_string(state), state_string(cur_state));
> +}
> +#define assert_dp_port_disabled(d) assert_dp_port((d), false)
> +
> +static void assert_edp_pll(struct drm_i915_private *dev_priv, bool state)
> +{
> + bool cur_state = I915_READ(DP_A) & DP_PLL_ENABLE;
> +
> + I915_STATE_WARN(cur_state != state,
> + "eDP PLL state assertion failure (expected %s, current
> %s)\n",
> + state_string(state), state_string(cur_state));
> +}
> +#define assert_edp_pll_enabled(d) assert_edp_pll((d), true)
> +#define assert_edp_pll_disabled(d) assert_edp_pll((d), false)
> +
> static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> - struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 dpa_ctl;
>
> - assert_pipe_disabled(dev_priv,
> - to_intel_crtc(crtc)->pipe);
> + assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_dp_port_disabled(intel_dp);
> + assert_edp_pll_disabled(dev_priv);
>
> DRM_DEBUG_KMS("\n");
> dpa_ctl = I915_READ(DP_A);
> - WARN(dpa_ctl & DP_PLL_ENABLE, "dp pll on, should be off\n");
> - WARN(dpa_ctl & DP_PORT_EN, "dp port still on, should be off\n");
>
> /* We don't adjust intel_dp->DP while tearing down the link, to
>* facilitate link retraining (e.g. after hotplug). Hence clear all
> @@ -2171,18 +2198,15 @@ static void ironlake_edp_pll_on(struct intel_dp
> *intel_dp)
> static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> - struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> - struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> u32 dpa_ctl;
>
> - assert_pipe_disabled(dev_priv,
> - to_intel_crtc(crtc)->pipe);
> + assert_pipe_disabled(dev_priv, crtc->pipe);
> + assert_dp_port_disabled(intel_dp);
> + assert_edp_pll_enabled(dev_priv);
>
> dpa_ctl = I915_READ(DP_A);
> - WARN((dpa_ctl & DP_PLL_ENABLE) == 0,
> - "dp pll off, should be on\n");
> - WARN(dpa_ctl & DP_PORT_EN, "dp port still on, should be off\n");
>
> /* We can't rely on the value tracked for the DP register in
>* intel_dp->DP because link_down must not change that (otherwise link
> --
> 2.4.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915/skl: Refuse to load outdated dmc firmware
There is known issue on GT interrupt delivery with DC6 and
firmwares <1.21. There is a suspicion that this causes
spurious gpu hangs on driver init and with some workloads,
as upgrading the firmware to 1.21 makes these problems
disappear.
As of now the current version included in distribution
firmware packages is very like to be 1.19. Play it safe and
refuse to load a firmware version that may affect gpu
side stability.
With < 1.23 there is a palette and dmc ram corruption issue
so blacklist anything below that.
v2: Refuse to load fw instead of notifying the user
v3: Rebase on header version changes
v4: Refuse to load anything less than 1.23
v5: Give enough information for user for finding correct fw (Chris)
v6: better url and formatting (Chris)
v7: move error log for each fail path (Mika)
bail out earlier in load path (Imre)
v8: Fix the version check (Imre)
Cc: Animesh Manna
Cc: Jani Nikula
Cc: Dave Gordon
Cc: Arun Siluvery
Cc: Imre Deak
Cc: Patrik Jakobsson
Cc: Rodrigo Vivi
Cc: Chris Wilson
References: https://01.org/linuxgraphics/downloads/skldmcver121
References: https://01.org/linuxgraphics/downloads/skylake-dmc-1.23
Testcase: igt/gem_exec_nop
Signed-off-by: Mika Kuoppala
Reviewed-by: Imre Deak
---
drivers/gpu/drm/i915/intel_csr.c | 34 --
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index e620e85..25b6ba7 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -47,6 +47,8 @@
MODULE_FIRMWARE(I915_CSR_SKL);
MODULE_FIRMWARE(I915_CSR_BXT);
+#define SKL_CSR_VERSION_REQUIRED CSR_VERSION(1, 23)
+
/*
* SKL CSR registers for DC5 and DC6
*/
@@ -303,10 +305,8 @@ static void finish_csr_load(const struct firmware *fw,
void *context)
uint32_t *dmc_payload;
bool fw_loaded = false;
- if (!fw) {
- i915_firmware_load_error_print(csr->fw_path, 0);
+ if (!fw)
goto out;
- }
if ((stepping == -ENODATA) || (substepping == -ENODATA)) {
DRM_ERROR("Unknown stepping info, firmware loading failed\n");
@@ -324,6 +324,17 @@ static void finish_csr_load(const struct firmware *fw,
void *context)
csr->version = css_header->version;
+ if (IS_SKYLAKE(dev) && csr->version < SKL_CSR_VERSION_REQUIRED) {
+ DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u,"
+" please upgrade to v%u.%u or later"
+"
[https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n";,
+CSR_VERSION_MAJOR(csr->version),
+CSR_VERSION_MINOR(csr->version),
+CSR_VERSION_MAJOR(SKL_CSR_VERSION_REQUIRED),
+CSR_VERSION_MINOR(SKL_CSR_VERSION_REQUIRED));
+ goto out;
+ }
+
readcount += sizeof(struct intel_css_header);
/* Extract Package Header information*/
@@ -405,17 +416,20 @@ static void finish_csr_load(const struct firmware *fw,
void *context)
intel_csr_load_program(dev);
fw_loaded = true;
- DRM_INFO("Finished loading %s (v%u.%u)\n",
-dev_priv->csr.fw_path,
-CSR_VERSION_MAJOR(csr->version),
-CSR_VERSION_MINOR(csr->version));
-
out:
- if (fw_loaded)
+ if (fw_loaded) {
intel_runtime_pm_put(dev_priv);
- else
+
+ DRM_INFO("Finished loading %s (v%u.%u)\n",
+dev_priv->csr.fw_path,
+CSR_VERSION_MAJOR(csr->version),
+CSR_VERSION_MINOR(csr->version));
+ } else {
intel_csr_load_status_set(dev_priv, FW_FAILED);
+ i915_firmware_load_error_print(csr->fw_path, 0);
+ }
+
release_firmware(fw);
}
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/14] drm/i915: s/DP_PLL_FREQ_160MHZ/DP_PLL_FREQ_162MHZ/
On Thu, Oct 29, 2015 at 09:25:59PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> The DP link frequency is 162MHz, not 160MHz. Rename the ILK eDP PLL
> defines to match.
>
> Signed-off-by: Ville Syrjälä
ocd ftw. Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 +-
> drivers/gpu/drm/i915/intel_dp.c | 10 +-
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8942532..d02e3c7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4199,7 +4199,7 @@ enum skl_disp_power_wells {
>
> /* eDP */
> #define DP_PLL_FREQ_270MHZ (0 << 16)
> -#define DP_PLL_FREQ_160MHZ (1 << 16)
> +#define DP_PLL_FREQ_162MHZ (1 << 16)
> #define DP_PLL_FREQ_MASK (3 << 16)
>
> /* locked once port is enabled */
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0b9b440..55d5246 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1557,11 +1557,11 @@ static void ironlake_set_pll_cpu_edp(struct intel_dp
> *intel_dp)
>
> if (crtc->config->port_clock == 162000) {
> /* For a long time we've carried around a ILK-DevA w/a for the
> - * 160MHz clock. If we're really unlucky, it's still required.
> + * 162MHz clock. If we're really unlucky, it's still required.
>*/
> - DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA
> w/a\n");
> - dpa_ctl |= DP_PLL_FREQ_160MHZ;
> - intel_dp->DP |= DP_PLL_FREQ_160MHZ;
> + DRM_DEBUG_KMS("162MHz cpu eDP clock, might need ilk devA
> w/a\n");
> + dpa_ctl |= DP_PLL_FREQ_162MHZ;
> + intel_dp->DP |= DP_PLL_FREQ_162MHZ;
> } else {
> dpa_ctl |= DP_PLL_FREQ_270MHZ;
> intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> @@ -2324,7 +2324,7 @@ static void intel_dp_get_config(struct intel_encoder
> *encoder,
> intel_dp_get_m_n(crtc, pipe_config);
>
> if (port == PORT_A) {
> - if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
> + if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_162MHZ)
> pipe_config->port_clock = 162000;
> else
> pipe_config->port_clock = 27;
> --
> 2.4.10
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
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 06/14] drm/i915: Check for FIFO underruns after modeset on IVB/HSW and CPT/PPT
On Thu, Oct 29, 2015 at 09:25:55PM +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Due to the shared error interrupt on IVB/HSW and CPT/PPT we may not
> always get an interrupt on a FIFO underrun. But we can always do an
> explicit check (like we do on GMCH platforms that have no underrun
> interrupt).
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 6 +-
> drivers/gpu/drm/i915/intel_drv.h | 3 +-
> drivers/gpu/drm/i915/intel_fifo_underrun.c | 121
> -
> 3 files changed, 105 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index c7cd9f7..e820147 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4719,9 +4719,9 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> if (IS_GEN2(dev))
> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>
> - /* Underruns don't raise interrupts, so check manually. */
> - if (HAS_GMCH_DISPLAY(dev))
> - i9xx_check_fifo_underruns(dev_priv);
> + /* Underruns don't always raise interrupts, so check manually. */
> + intel_check_cpu_fifo_underruns(dev_priv);
> + intel_check_pch_fifo_underruns(dev_priv);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1a3bbdc..72cc272 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -959,7 +959,8 @@ void intel_cpu_fifo_underrun_irq_handler(struct
> drm_i915_private *dev_priv,
>enum pipe pipe);
> void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>enum transcoder pch_transcoder);
> -void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv);
> +void intel_check_cpu_fifo_underruns(struct drm_i915_private *dev_priv);
> +void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
>
> /* i915_irq.c */
> void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index 54daa66..a546fc3 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -85,37 +85,28 @@ static bool cpt_can_enable_serr_int(struct drm_device
> *dev)
> }
>
> /**
> - * i9xx_check_fifo_underruns - check for fifo underruns
> - * @dev_priv: i915 device instance
> + * intel_check_cpu_fifo_underruns - check for fifo underruns
> + * @crtc: pipe
> *
> * This function checks for fifo underruns on GMCH platforms. This needs to
> be
> * done manually on modeset to make sure that we catch all underruns since
> they
> * do not generate an interrupt by themselves on these platforms.
> */
Stale kerneldoc above, just delete it. With that fixed:
Reviewed-by: Daniel Vetter
> -void i9xx_check_fifo_underruns(struct drm_i915_private *dev_priv)
> +static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
> {
> - struct intel_crtc *crtc;
> -
> - spin_lock_irq(&dev_priv->irq_lock);
> -
> - for_each_intel_crtc(dev_priv->dev, crtc) {
> - u32 reg = PIPESTAT(crtc->pipe);
> - u32 pipestat;
> -
> - if (crtc->cpu_fifo_underrun_disabled)
> - continue;
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + u32 reg = PIPESTAT(crtc->pipe);
> + u32 pipestat = I915_READ(reg) & 0x;
>
> - pipestat = I915_READ(reg) & 0x;
> - if ((pipestat & PIPE_FIFO_UNDERRUN_STATUS) == 0)
> - continue;
> + assert_spin_locked(&dev_priv->irq_lock);
>
> - I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
> - POSTING_READ(reg);
> + if ((pipestat & PIPE_FIFO_UNDERRUN_STATUS) == 0)
> + return;
>
> - DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
> - }
> + I915_WRITE(reg, pipestat | PIPE_FIFO_UNDERRUN_STATUS);
> + POSTING_READ(reg);
>
> - spin_unlock_irq(&dev_priv->irq_lock);
> + DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
> }
>
> static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
> @@ -150,6 +141,23 @@ static void ironlake_set_fifo_underrun_reporting(struct
> drm_device *dev,
> ironlake_disable_display_irq(dev_priv, bit);
> }
>
> +static void ivybridge_check_fifo_underruns(struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> + uint32_t err_int = I915_READ(GEN7_ERR_INT);
> +
> + assert_spin_locked(&dev_priv->irq_lock);
> +
> + if ((err_int & ERR_INT_FIFO_UNDERRUN(pipe)) == 0)
> + return;
> +
> + I915_WRITE(
Re: [Intel-gfx] [PATCH 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
On Thu, Oct 29, 2015 at 11:21:28PM +0200, Ville Syrjälä wrote: > On Thu, Oct 29, 2015 at 05:57:57PM -0200, Paulo Zanoni wrote: > > 2015-10-29 17:25 GMT-02:00 : > > > From: Ville Syrjälä > > > > > > We get spurious PCH FIFO underruns if we enable the reporting too soon > > > after enabling the crtc. Move it to be the last step, after the encoder > > > enable. Additionally we need an extra vblank wait, otherwise we still > > > get the underruns. Presumably the pipe/fdi isn't yet fully up and running > > > otherwise. > > > > > > For symmetry, disable the PCH underrun reporting as the first thing, > > > just before encoder disable, when shutting down the crtc. > > > > Is there any place that describes where/when a FIFO underrun is > > expected and where/when one is an actual problem that needs to be > > solved? How do we know the underruns avoided by these patch are not a > > signal of real bugs? > > Can't be 100% sure since its not documented anywhere. But we've been > getting these since forever now and stuff still works (more or less at > least), so I'm inclined to say we don't have to care about them. Also > in these case we only get PCH FIFO underruns and no CPU pipe > underruns, so I'm tempted to say it's not that serious. > > IIRC I once tracked some of these down to having the FDI PLL enabled > with FDI RX/TX disabled. Or something like that, but don't quote me > on that since my memory might be failing here. Obviously that can't > explain it all since I still need the vblank wait to eliminate them. > Anyway, this time I didn't try to narrow it down too much. Instead > my aim was more to reliably eliminate them without permamently disabling > the underrun detection. > > In any case, we can't get the bat stuff really working until we get > the results to be stable, and these underruns are one big obstacle > to that. Agreed with Ville here, these are old platforms and we want stable BAT results first. On gen9+ we should try a bit harder, or at least make a note in JIRA that we need to look into this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation 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 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
On Fri, Oct 30, 2015 at 02:08:51PM +0200, Ville Syrjälä wrote:
> On Fri, Oct 30, 2015 at 12:06:09PM +0200, Jani Nikula wrote:
> > On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> > > From: Ville Syrjälä
> > >
> > > We get spurious PCH FIFO underruns if we enable the reporting too soon
> > > after enabling the crtc. Move it to be the last step, after the encoder
> > > enable. Additionally we need an extra vblank wait, otherwise we still
> > > get the underruns. Presumably the pipe/fdi isn't yet fully up and running
> > > otherwise.
> > >
> > > For symmetry, disable the PCH underrun reporting as the first thing,
> > > just before encoder disable, when shutting down the crtc.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 13 +
> > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 99fb33f..d5cb899 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc
> > > *crtc)
> > > intel_crtc->active = true;
> > >
> > > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> > >
> > > for_each_encoder_on_crtc(dev, crtc, encoder)
> > > if (encoder->pre_enable)
> > > @@ -4912,6 +4911,12 @@ static void ironlake_crtc_enable(struct drm_crtc
> > > *crtc)
> > >
> > > if (HAS_PCH_CPT(dev))
> > > cpt_verify_modeset(dev, intel_crtc->pipe);
> > > +
> > > + if (intel_crtc->config->has_pch_encoder) {
> > > + /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > > + intel_wait_for_vblank(dev, pipe);
> > > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> >
> > Nitpick, moving this within the if (has_pch_encoder) isn't documented in
> > the commit message. Does that change have an impact?
>
> I don't much of a real concern here. I think the following might
> happen (all on the same pipe):
>
> 1. enable PCH port
> 2. disable PCH port
> 3. PCH FIFO underrun just after we've re-enabled the PCH
>underrun reporting
> 4. enable port A
> 5. PCH FIFO underrun reporting isn't enabled anymore for this pipe
>
> But since it's driving a non-PCH port anyway, that doesn't seem like
> a huge worry. But I suppose I could change it to always enable PCH
> FIFO underrun reporting even for port A. It should do no harm at least.
Iirc we still fail to enable fifo underrun reporting with fastboot (should
fix this now since we update watermarks on takeover). That was the reason
to unconditionally enable fifo underruns even on the pch, to make it work
on platforms where the pch interrupt source is shared. See the pile of
hurt at the end of intel_sanitize_crtc.
I'd just keep it out of the if for now.
-Daniel
>
> >
> > BR,
> > Jani.
> >
> > > + }
> > > }
> > >
> > > /* IPS only exists on ULT machines and is tied to pipe A. */
> > > @@ -5040,15 +5045,15 @@ static void ironlake_crtc_disable(struct drm_crtc
> > > *crtc)
> > > int pipe = intel_crtc->pipe;
> > > u32 reg, temp;
> > >
> > > + if (intel_crtc->config->has_pch_encoder)
> > > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > > +
> > > for_each_encoder_on_crtc(dev, crtc, encoder)
> > > encoder->disable(encoder);
> > >
> > > drm_crtc_vblank_off(crtc);
> > > assert_vblank_disabled(crtc);
> > >
> > > - if (intel_crtc->config->has_pch_encoder)
> > > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > > -
> > > intel_disable_pipe(intel_crtc);
> > >
> > > ironlake_pfit_disable(intel_crtc, false);
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
>
> --
> 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
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: Allow unready gpu to be reset on gen8
On Fri, Oct 30, 2015 at 05:18:18PM +0200, Mika Kuoppala wrote: > Chris Wilson writes: > > > On Fri, Oct 30, 2015 at 04:43:49PM +0200, Mika Kuoppala wrote: > >> Gen9 has had demonstrated cases where forcing a not ready gpu > >> into reset has caused system hang [1]. > >> > >> Gen8 has never to this date demonstrated such behaviour. > >> > >> In our CI tests bsw sometimes ends up in a state where it claims it > >> is not ready for reset, based on reset request, after gpu hang. > >> > >> Allow gen8 to reset even after claims of nonreadiness in order > >> to keep the gpu accessible. Enhance logging so that it will be > >> clear what conditions led to decision of proceeding or bailing out, > >> so that we will spot if this way of forcing our will against gpu turns > >> out to be foolhardy. > >> > >> References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959 > >> Cc: Daniel Vetter > >> Cc: Tomi Sarvela > >> Signed-off-by: Mika Kuoppala > >> --- > >> drivers/gpu/drm/i915/intel_uncore.c | 9 - > >> 1 file changed, 8 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_uncore.c > >> b/drivers/gpu/drm/i915/intel_uncore.c > >> index f0f97b2..47c17f2 100644 > >> --- a/drivers/gpu/drm/i915/intel_uncore.c > >> +++ b/drivers/gpu/drm/i915/intel_uncore.c > >> @@ -1504,7 +1504,14 @@ not_ready: > >>I915_WRITE(RING_RESET_CTL(engine->mmio_base), > >> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET)); > >> > >> - return -EIO; > > > > Where's the reference for where we hit this EIO on gen8? > > > > Internal CI logs, relevant part cutpasted below. If you want > full log holler me in irc. So you are saying that's there no bugzilla for this... :-p -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: Allow unready gpu to be reset on gen8
Chris Wilson writes:
> On Fri, Oct 30, 2015 at 04:43:49PM +0200, Mika Kuoppala wrote:
>> Gen9 has had demonstrated cases where forcing a not ready gpu
>> into reset has caused system hang [1].
>>
>> Gen8 has never to this date demonstrated such behaviour.
>>
>> In our CI tests bsw sometimes ends up in a state where it claims it
>> is not ready for reset, based on reset request, after gpu hang.
>>
>> Allow gen8 to reset even after claims of nonreadiness in order
>> to keep the gpu accessible. Enhance logging so that it will be
>> clear what conditions led to decision of proceeding or bailing out,
>> so that we will spot if this way of forcing our will against gpu turns
>> out to be foolhardy.
>>
>> References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959
>> Cc: Daniel Vetter
>> Cc: Tomi Sarvela
>> Signed-off-by: Mika Kuoppala
>> ---
>> drivers/gpu/drm/i915/intel_uncore.c | 9 -
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index f0f97b2..47c17f2 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1504,7 +1504,14 @@ not_ready:
>> I915_WRITE(RING_RESET_CTL(engine->mmio_base),
>> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
>>
>> -return -EIO;
>
> Where's the reference for where we hit this EIO on gen8?
>
Internal CI logs, relevant part cutpasted below. If you want
full log holler me in irc.
[ 119.147727] kms_pipe_crc_basic: starting subtest hang-read-crc-pipe-A
[ 124.785063] [drm] stuck on render ring
[ 124.800850] [drm] GPU HANG: ecode 8:0:0xfffe, in kms_pipe_crc_ba
[5590], reason: Ring hung, action: reset
[ 124.801154] [drm] GPU hangs can indicate a bug anywhere in the entire
gfx stack, including userspace.
[ 124.801161] [drm] Please file a _new_ bug report on
bugs.freedesktop.org against DRI -> DRM/Intel
[ 124.801167] [drm] drm/i915 developers can then reassign to the right
component if it's not a kernel issue.
[ 124.801173] [drm] The gpu crash dump is required to analyze gpu
hangs, so please always attach it.
[ 124.801179] [drm] GPU crash dump saved to /sys/class/drm/card0/error
[ 124.801785] kobject: 'card0' (880174ad92a0): kobject_uevent_env
[ 124.801940] kobject: 'card0' (880174ad92a0): fill_kobj_path: path
= '/devices/pci:00/:00:02.0/drm/card0'
[ 124.805032] kobject: 'card0' (880174ad92a0): kobject_uevent_env
[ 124.805089] kobject: 'card0' (880174ad92a0): fill_kobj_path: path
= '/devices/pci:00/:00:02.0/drm/card0'
[ 125.511744] [drm:gen8_do_reset [i915]] *ERROR* render ring: reset
request timeout
[ 125.511922] [drm] Simulated gpu hang, resetting stop_rings
[ 125.511927] drm/i915: Resetting chip after gpu hang
[ 125.511954] [drm:i915_reset [i915]] *ERROR* Failed to reset chip: -5
[ 125.637612] kms_pipe_crc_basic: exiting, ret=0
[ 125.653608] [drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring
create req: -5
[ 125.847695] gem_ctx_param_basic: executing
[ 125.850086] [drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring
create req: -5
[ 125.854482] gem_ctx_param_basic: exiting, ret=99
[ 126.038693] kms_addfb_basic: executing
[ 126.041754] [drm:intel_lr_context_deferred_alloc [i915]] *ERROR* ring
create req: -5
-Mika
>> +if (INTEL_INFO(dev)->gen == 9) {
>> +DRM_ERROR("Reset would risk system stability, bailing out\n");
>> +return -EIO;
>> +}
>> +
>> +DRM_ERROR("Forcing non ready gpu into reset\n");
>> +
>> +return gen6_do_reset(dev);
>> }
>>
>> static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device
>> *)
>> --
>> 2.5.0
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> 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: Allow unready gpu to be reset on gen8
On Fri, Oct 30, 2015 at 04:43:49PM +0200, Mika Kuoppala wrote:
> Gen9 has had demonstrated cases where forcing a not ready gpu
> into reset has caused system hang [1].
>
> Gen8 has never to this date demonstrated such behaviour.
>
> In our CI tests bsw sometimes ends up in a state where it claims it
> is not ready for reset, based on reset request, after gpu hang.
>
> Allow gen8 to reset even after claims of nonreadiness in order
> to keep the gpu accessible. Enhance logging so that it will be
> clear what conditions led to decision of proceeding or bailing out,
> so that we will spot if this way of forcing our will against gpu turns
> out to be foolhardy.
>
> References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959
> Cc: Daniel Vetter
> Cc: Tomi Sarvela
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index f0f97b2..47c17f2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1504,7 +1504,14 @@ not_ready:
> I915_WRITE(RING_RESET_CTL(engine->mmio_base),
> _MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
>
> - return -EIO;
Where's the reference for where we hit this EIO on gen8?
> + if (INTEL_INFO(dev)->gen == 9) {
> + DRM_ERROR("Reset would risk system stability, bailing out\n");
> + return -EIO;
> + }
> +
> + DRM_ERROR("Forcing non ready gpu into reset\n");
> +
> + return gen6_do_reset(dev);
> }
>
> static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device
> *)
> --
> 2.5.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] drm/i915: Allow unready gpu to be reset on gen8
Gen9 has had demonstrated cases where forcing a not ready gpu
into reset has caused system hang [1].
Gen8 has never to this date demonstrated such behaviour.
In our CI tests bsw sometimes ends up in a state where it claims it
is not ready for reset, based on reset request, after gpu hang.
Allow gen8 to reset even after claims of nonreadiness in order
to keep the gpu accessible. Enhance logging so that it will be
clear what conditions led to decision of proceeding or bailing out,
so that we will spot if this way of forcing our will against gpu turns
out to be foolhardy.
References [1]: https://bugs.freedesktop.org/show_bug.cgi?id=89959
Cc: Daniel Vetter
Cc: Tomi Sarvela
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index f0f97b2..47c17f2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1504,7 +1504,14 @@ not_ready:
I915_WRITE(RING_RESET_CTL(engine->mmio_base),
_MASKED_BIT_DISABLE(RESET_CTL_REQUEST_RESET));
- return -EIO;
+ if (INTEL_INFO(dev)->gen == 9) {
+ DRM_ERROR("Reset would risk system stability, bailing out\n");
+ return -EIO;
+ }
+
+ DRM_ERROR("Forcing non ready gpu into reset\n");
+
+ return gen6_do_reset(dev);
}
static int (*intel_get_gpu_reset(struct drm_device *dev))(struct drm_device *)
--
2.5.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 2/3 v3] Unify handling of slow/combinatorial tests
On Fri, Oct 30, 2015 at 03:18:30PM +0200, David Weinehall wrote:
> @@ -931,16 +930,20 @@ run_basic_modes(const struct access_mode *mode,
> struct buffers buffers;
>
> for (h = hangs; h->suffix; h++) {
> - if (!all && *h->suffix)
> - continue;
> + unsigned int subtest_flags;
>
> - for (p = all ? pipelines : pskip; p->prefix; p++) {
> + if (*h->suffix)
> + subtest_flags = SUBTEST_TYPE_SLOW;
They aren't all slow though. The hang tests are (because it takes a long
time for a hang to occur and we need to race many times for reasonable
coverage). Many of the tests here were being skipped because QA couldn't
handle the full set.
Now that you have flags, adding h->flags would be better than heuristic
based on h->suffix. Similary we can use p->flags, then we get
igt_subtest_flags_f(h->flags | p->flags, "foo"); And if we have more
faith in QA in being able to dtrt we really only need to mark the known
slow cases (many the hang injection) as being truly slow.
-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 00/14] drm/i915: FIFO underrun elimination for PCH platforms
On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> This series eliminates all spurious PCH FIFO underrun reports on my
> machines during a BAT run ('-t basic -x reload -x suspend' actually).
> It also eliminates the non-spurious but expected underrun reports
> on ILK.
>
> I also embarked on a small scale cleanup of the CPU eDP PLL code while
> I was trying to get to the bottom of the underruns on ILK. So I figured
> I'd include that work here as well.
>
> I've tested this on ILK, SNB, two IVBs, and one HSW, with as many
> displays plugged in as possible. What I could actually test is HSW/BDW
> CRT output since I have no machine for that.
>
> The series is available here:
> git://github.com/vsyrjala/linux.git pch_fifo_underrun_fix_4
I spammed "a few" potentially relevant bugs with test requests:
https://bugs.freedesktop.org/show_bug.cgi?id=74102
https://bugs.freedesktop.org/show_bug.cgi?id=85906
https://bugs.freedesktop.org/show_bug.cgi?id=88977
https://bugs.freedesktop.org/show_bug.cgi?id=89518
https://bugs.freedesktop.org/show_bug.cgi?id=89806
https://bugs.freedesktop.org/show_bug.cgi?id=90239
https://bugs.freedesktop.org/show_bug.cgi?id=90307
BR,
Jani.
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory
Chris Wilson writes:
> Ringbuffers are now being written to either through LLC or WC paths, so
> treating them as simply iomem is no longer adequate. However, for the
> older !llc hardware, the hardware is documentated as treating the TAIL
> register update as serialising, so we can relax the barriers when filling
> the rings (but even if it were not, it is still an uncached register write
> and so serialising anyway.).
>
> For simplicity, let's ignore the iomem annotation.
>
> v2: Remove iomem from ringbuffer->virtual_address
>
iomem annotation is for sparse. i915_irq.c
still uses ioread32 thus mixing the address spaces.
I see this has already r-b but something like this
as a followup would make sparse quiet:
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 6e0a568..b5e9383 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2831,7 +2831,7 @@ semaphore_waits_for(struct intel_engine_cs *ring,
u32 *seqno)
head &= ring->buffer->size - 1;
/* This here seems to blow up */
- cmd = ioread32(ring->buffer->virtual_start + head);
+ cmd = intel_ringbuffer_read(ring->buffer, head);
if (cmd == ipehr)
break;
@@ -2841,11 +2841,11 @@ semaphore_waits_for(struct intel_engine_cs
*ring, u32 *seqno)
if (!i)
return NULL;
- *seqno = ioread32(ring->buffer->virtual_start + head + 4) + 1;
+ *seqno = intel_ringbuffer_read(ring->buffer, head + 4) + 1;
if (INTEL_INFO(ring->dev)->gen >= 8) {
- offset = ioread32(ring->buffer->virtual_start + head + 12);
+ offset = intel_ringbuffer_read(ring->buffer, head +
12);
offset <<= 32;
- offset = ioread32(ring->buffer->virtual_start +
head + 8);
+ offset = intel_ringbuffer_read(ring->buffer, head + 8);
}
return semaphore_wait_to_signaller_ring(ring, ipehr, offset);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ea6f8a7..48fdfc3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1999,7 +1999,7 @@ void intel_unpin_ringbuffer_obj(struct
intel_ringbuffer *ringbuf)
if (HAS_LLC(ringbuf->obj->base.dev) &&
!ringbuf->obj->stolen)
vunmap(ringbuf->virtual_start);
else
- iounmap(ringbuf->virtual_start);
+ iounmap((void __iomem
*)ringbuf->virtual_start);
ringbuf->virtual_start = NULL;
i915_gem_object_ggtt_unpin(ringbuf->obj);
}
@@ -2059,7 +2059,7 @@ int intel_pin_and_map_ringbuffer_obj(struct
drm_device *dev,
return ret;
}
- ringbuf->virtual_start =
ioremap_wc(dev_priv->gtt.mappable_base +
+
ringbuf->virtual_start = (void __force
*)ioremap_wc(dev_priv->gtt.mappable_base +
i915_gem_obj_ggtt_offset(obj),
ringbuf->size);
if (ringbuf->virtual_start == NULL) {
i915_gem_object_ggtt_unpin(obj);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a15f6c2..335e632 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -448,6 +448,13 @@ static inline void intel_ringbuffer_advance(struct
intel_ringbuffer *rb)
rb->tail &= rb->size - 1;
}
+static inline u32 intel_ringbuffer_read(const struct intel_ringbuffer
*rb,
+ const u32 offset)
+{
+ WARN_ON(offset >= rb->size);
+ return *(uint32_t *)(rb->virtual_start + offset);
+}
+
static inline void intel_ring_emit(struct intel_engine_cs *ring,
u32 data)
{
-Mika
> Signed-off-by: Chris Wilson
> Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_lrc.c| 7 +--
> drivers/gpu/drm/i915/intel_lrc.h| 6 +++---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +--
> drivers/gpu/drm/i915/intel_ringbuffer.h | 19 +--
> 4 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index d38746c5370d..10020505be75 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -713,13 +713,8 @@ intel_logical_ring_advance_and_submit(struct
> drm_i915_gem_request *request)
>
> static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
> {
> - uint32_t __iomem *virt;
> int rem = ringbuf
[Intel-gfx] [PATCH i-g-t 0/3 v3] Unify slow/combinatorial test handling
Until now we've had no unified way to handle slow/combinatorial tests. Most of the time we don't want to run slow/combinatorial tests, so this should remain the default, but when we do want to run such tests, it has been handled differently in different tests. This patch adds an --all command line option to igt_core, changes gem_concurrent_blit and kms_frontbuffer_tracking to use this instead of their own methods, and removes gem_concurrent_all in the process, since it's now unnecessary. Test cases that have subtests that should not be run by default should use the igt_subtest_flags() / ugt_subtest_flags_f() functions and pass the subtest types as part of the flags parameter. v2: Incorporate various suggestions from reviewers. v3: Rewrite to provide a generic mechanism for categorising the subtests David Weinehall (3): Copy gem_concurrent_all to gem_concurrent_blit Unify handling of slow/combinatorial tests Remove superfluous gem_concurrent_all.c lib/igt_core.c | 43 +- lib/igt_core.h | 42 ++ tests/.gitignore |1 - tests/Makefile.sources |1 - tests/gem_concurrent_all.c | 1108 - tests/gem_concurrent_blit.c | 1114 +- tests/kms_frontbuffer_tracking.c | 207 +++ 7 files changed, 1300 insertions(+), 1216 deletions(-) delete mode 100644 tests/gem_concurrent_all.c -- 2.6.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3 v3] Unify handling of slow/combinatorial tests
Some subtests are not run by default, for various reasons;
be it because they're only for debugging, because they're slow,
or because they are not of high enough quality.
This patch aims to introduce a common mechanism for categorising
the subtests and introduces a flag (--all) that runs/lists all
subtests instead of just the default set.
Signed-off-by: David Weinehall
---
lib/igt_core.c | 43 ++--
lib/igt_core.h | 42
tests/gem_concurrent_blit.c | 50 +-
tests/kms_frontbuffer_tracking.c | 207 ++-
4 files changed, 218 insertions(+), 124 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 59127cafe606..2034bc33ad78 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -216,6 +216,7 @@ const char *igt_interactive_debug;
/* subtests helpers */
static bool list_subtests = false;
+static unsigned int subtest_types_mask = SUBTEST_TYPE_NORMAL;
static char *run_single_subtest = NULL;
static bool run_single_subtest_found = false;
static const char *in_subtest = NULL;
@@ -234,12 +235,13 @@ int test_children_sz;
bool test_child;
enum {
- OPT_LIST_SUBTESTS,
- OPT_RUN_SUBTEST,
- OPT_DESCRIPTION,
- OPT_DEBUG,
- OPT_INTERACTIVE_DEBUG,
- OPT_HELP = 'h'
+ OPT_LIST_SUBTESTS,
+ OPT_WITH_ALL_SUBTESTS,
+ OPT_RUN_SUBTEST,
+ OPT_DESCRIPTION,
+ OPT_DEBUG,
+ OPT_INTERACTIVE_DEBUG,
+ OPT_HELP = 'h'
};
static int igt_exitcode = IGT_EXIT_SUCCESS;
@@ -478,6 +480,7 @@ static void print_usage(const char *help_str, bool
output_on_stderr)
fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
fprintf(f, " --list-subtests\n"
+ " --all\n"
" --run-subtest \n"
" --debug[=log-domain]\n"
" --interactive-debug[=domain]\n"
@@ -510,6 +513,7 @@ static int common_init(int *argc, char **argv,
int c, option_index = 0, i, x;
static struct option long_options[] = {
{"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
+ {"all", 0, 0, OPT_WITH_ALL_SUBTESTS},
{"run-subtest", 1, 0, OPT_RUN_SUBTEST},
{"help-description", 0, 0, OPT_DESCRIPTION},
{"debug", optional_argument, 0, OPT_DEBUG},
@@ -617,6 +621,10 @@ static int common_init(int *argc, char **argv,
if (!run_single_subtest)
list_subtests = true;
break;
+ case OPT_WITH_ALL_SUBTESTS:
+ if (!run_single_subtest)
+ subtest_types_mask = SUBTEST_TYPE_ALL;
+ break;
case OPT_RUN_SUBTEST:
if (!list_subtests)
run_single_subtest = strdup(optarg);
@@ -1629,6 +1637,29 @@ void igt_skip_on_simulation(void)
igt_require(!igt_run_in_simulation());
}
+/**
+ * igt_match_subtest_flags:
+ *
+ * This function is used to check whether the attributes of the subtest
+ * makes it a candidate for inclusion in the test run; this is used to
+ * categorise tests, for instance to exclude tests that are purely for
+ * debug purposes, tests that are specific to certain environments,
+ * or tests that are very slow.
+ *
+ * Note that a test has to have all its flags met to be run; for instance
+ * a subtest with the flags SUBTEST_TYPE_SLOW | SUBTEST_TYPE_DEBUG requires
+ * "--subtest-types=slow,debug" or "--all" to be executed
+ *
+ * @subtest_flags: The subtests to check for
+ *
+ * Returns: true if the subtest test should be run,
+ * false if the subtest should be skipped
+ */
+bool igt_match_subtest_flags(unsigned long subtest_flags)
+{
+ return ((subtest_flags & subtest_types_mask) == subtest_flags);
+}
+
/* structured logging */
/**
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 5ae09653fd55..495cb77a8aea 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -191,6 +191,48 @@ bool __igt_run_subtest(const char *subtest_name);
#define igt_subtest_f(f...) \
__igt_subtest_f(igt_tokencat(__tmpchar, __LINE__), f)
+enum {
+ /* The set of tests run if nothing else is specified */
+ SUBTEST_TYPE_NORMAL = 1 << 0,
+ /* Basic Acceptance Testing set */
+ SUBTEST_TYPE_BASIC = 1 << 1,
+ /* Tests that are very slow */
+ SUBTEST_TYPE_SLOW = 1 << 2,
+ /* Tests that mainly intended for debugging */
+ SUBTEST_TYPE_DEBUG = 1 << 3,
+ SUBTEST_TYPE_ALL = ~0
+} subtest_types;
+
+bool igt_match_subtest_flags(unsigned long subtest_flags);
+
+/**
+ * igt_subtest_flags:
+ * @name: name of the subtest
+ * @__subtest_flags: the categories the subtest belongs to
+ *
+ * This is a wrapper around igt_subtest that will only execute the
+ * testcase if all of the flags passed to this function match those
+ * specified by the list of subtest categories passed from the
+ * command line;
[Intel-gfx] [PATCH i-g-t 3/3 v3] Remove superfluous gem_concurrent_all.c
When gem_concurrent_blit was converted to use the new common framework
for choosing whether or not to include slow/combinatorial tests,
gem_concurrent_all became superfluous. This patch removes it.
Signed-off-by: David Weinehall
---
tests/.gitignore |1 -
tests/Makefile.sources |1 -
tests/gem_concurrent_all.c | 1108
3 files changed, 1110 deletions(-)
delete mode 100644 tests/gem_concurrent_all.c
diff --git a/tests/.gitignore b/tests/.gitignore
index beda5117da5c..da4f9961fc60 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -23,7 +23,6 @@ gem_bad_reloc
gem_basic
gem_caching
gem_close_race
-gem_concurrent_all
gem_concurrent_blit
gem_cpu_reloc
gem_cs_prefetch
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index ac731f90dcb2..321c7f33e4d3 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -14,7 +14,6 @@ TESTS_progs_M = \
gem_caching \
gem_close_race \
gem_concurrent_blit \
- gem_concurrent_all \
gem_cs_tlb \
gem_ctx_param_basic \
gem_ctx_bad_exec \
diff --git a/tests/gem_concurrent_all.c b/tests/gem_concurrent_all.c
deleted file mode 100644
index 1d2d787202df..
--- a/tests/gem_concurrent_all.c
+++ /dev/null
@@ -1,1108 +0,0 @@
-/*
- * Copyright © 2009,2012,2013 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:
- *Eric Anholt
- *Chris Wilson
- *Daniel Vetter
- *
- */
-
-/** @file gem_concurrent.c
- *
- * This is a test of pread/pwrite/mmap behavior when writing to active
- * buffers.
- *
- * Based on gem_gtt_concurrent_blt.
- */
-
-#include "igt.h"
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include
-
-#include "intel_bufmgr.h"
-
-IGT_TEST_DESCRIPTION("Test of pread/pwrite/mmap behavior when writing to
active"
-" buffers.");
-
-int fd, devid, gen;
-struct intel_batchbuffer *batch;
-int all;
-
-static void
-nop_release_bo(drm_intel_bo *bo)
-{
- drm_intel_bo_unreference(bo);
-}
-
-static void
-prw_set_bo(drm_intel_bo *bo, uint32_t val, int width, int height)
-{
- int size = width * height, i;
- uint32_t *tmp;
-
- tmp = malloc(4*size);
- if (tmp) {
- for (i = 0; i < size; i++)
- tmp[i] = val;
- drm_intel_bo_subdata(bo, 0, 4*size, tmp);
- free(tmp);
- } else {
- for (i = 0; i < size; i++)
- drm_intel_bo_subdata(bo, 4*i, 4, &val);
- }
-}
-
-static void
-prw_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo
*tmp)
-{
- int size = width * height, i;
- uint32_t *vaddr;
-
- do_or_die(drm_intel_bo_map(tmp, true));
- do_or_die(drm_intel_bo_get_subdata(bo, 0, 4*size, tmp->virtual));
- vaddr = tmp->virtual;
- for (i = 0; i < size; i++)
- igt_assert_eq_u32(vaddr[i], val);
- drm_intel_bo_unmap(tmp);
-}
-
-static drm_intel_bo *
-unmapped_create_bo(drm_intel_bufmgr *bufmgr, int width, int height)
-{
- drm_intel_bo *bo;
-
- bo = drm_intel_bo_alloc(bufmgr, "bo", 4*width*height, 0);
- igt_assert(bo);
-
- return bo;
-}
-
-static drm_intel_bo *
-snoop_create_bo(drm_intel_bufmgr *bufmgr, int width, int height)
-{
- drm_intel_bo *bo;
-
- igt_skip_on(gem_has_llc(fd));
-
- bo = unmapped_create_bo(bufmgr, width, height);
- gem_set_caching(fd, bo->handle, I915_CACHING_CACHED);
- drm_intel_bo_disable_reuse(bo);
-
- return bo;
-}
-
-static void
-gtt_set_bo(drm_intel_bo *bo, uint32_t val, int width, int height)
-{
- uint32_t *vaddr = bo->virtual;
- int size = width * height;
-
- drm_intel_gem_bo_start_gtt_access(bo, true);
- while (size--)
- *vaddr++ = val;
-
[Intel-gfx] [PATCH i-g-t 1/3 v3] Copy gem_concurrent_all to gem_concurrent_blit
We'll both rename gem_concurrent_all over gem_concurrent_blit
and change gem_concurrent_blit in this changeset. To make
this easier to follow we first do the the rename.
Signed-off-by: David Weinehall
---
tests/gem_concurrent_blit.c | 1116 ++-
1 file changed, 1108 insertions(+), 8 deletions(-)
diff --git a/tests/gem_concurrent_blit.c b/tests/gem_concurrent_blit.c
index 513de4a1b719..1d2d787202df 100644
--- a/tests/gem_concurrent_blit.c
+++ b/tests/gem_concurrent_blit.c
@@ -1,8 +1,1108 @@
-/* This test is just a duplicate of gem_concurrent_all. */
-/* However the executeable will be gem_concurrent_blit. */
-/* The main function examines argv[0] and, in the case */
-/* of gem_concurent_blit runs only a subset of the */
-/* available subtests. This avoids the use of */
-/* non-standard command line parameters which can cause */
-/* problems for automated testing */
-#include "gem_concurrent_all.c"
+/*
+ * Copyright © 2009,2012,2013 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:
+ *Eric Anholt
+ *Chris Wilson
+ *Daniel Vetter
+ *
+ */
+
+/** @file gem_concurrent.c
+ *
+ * This is a test of pread/pwrite/mmap behavior when writing to active
+ * buffers.
+ *
+ * Based on gem_gtt_concurrent_blt.
+ */
+
+#include "igt.h"
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test of pread/pwrite/mmap behavior when writing to
active"
+" buffers.");
+
+int fd, devid, gen;
+struct intel_batchbuffer *batch;
+int all;
+
+static void
+nop_release_bo(drm_intel_bo *bo)
+{
+ drm_intel_bo_unreference(bo);
+}
+
+static void
+prw_set_bo(drm_intel_bo *bo, uint32_t val, int width, int height)
+{
+ int size = width * height, i;
+ uint32_t *tmp;
+
+ tmp = malloc(4*size);
+ if (tmp) {
+ for (i = 0; i < size; i++)
+ tmp[i] = val;
+ drm_intel_bo_subdata(bo, 0, 4*size, tmp);
+ free(tmp);
+ } else {
+ for (i = 0; i < size; i++)
+ drm_intel_bo_subdata(bo, 4*i, 4, &val);
+ }
+}
+
+static void
+prw_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo
*tmp)
+{
+ int size = width * height, i;
+ uint32_t *vaddr;
+
+ do_or_die(drm_intel_bo_map(tmp, true));
+ do_or_die(drm_intel_bo_get_subdata(bo, 0, 4*size, tmp->virtual));
+ vaddr = tmp->virtual;
+ for (i = 0; i < size; i++)
+ igt_assert_eq_u32(vaddr[i], val);
+ drm_intel_bo_unmap(tmp);
+}
+
+static drm_intel_bo *
+unmapped_create_bo(drm_intel_bufmgr *bufmgr, int width, int height)
+{
+ drm_intel_bo *bo;
+
+ bo = drm_intel_bo_alloc(bufmgr, "bo", 4*width*height, 0);
+ igt_assert(bo);
+
+ return bo;
+}
+
+static drm_intel_bo *
+snoop_create_bo(drm_intel_bufmgr *bufmgr, int width, int height)
+{
+ drm_intel_bo *bo;
+
+ igt_skip_on(gem_has_llc(fd));
+
+ bo = unmapped_create_bo(bufmgr, width, height);
+ gem_set_caching(fd, bo->handle, I915_CACHING_CACHED);
+ drm_intel_bo_disable_reuse(bo);
+
+ return bo;
+}
+
+static void
+gtt_set_bo(drm_intel_bo *bo, uint32_t val, int width, int height)
+{
+ uint32_t *vaddr = bo->virtual;
+ int size = width * height;
+
+ drm_intel_gem_bo_start_gtt_access(bo, true);
+ while (size--)
+ *vaddr++ = val;
+}
+
+static void
+gtt_cmp_bo(drm_intel_bo *bo, uint32_t val, int width, int height, drm_intel_bo
*tmp)
+{
+ uint32_t *vaddr = bo->virtual;
+ int y;
+
+ /* GTT access is slow. So we just compare a few points */
+ drm_intel_gem_bo_start_gtt_access(bo, false);
+ for (y = 0; y < height; y++)
+ igt_assert_eq_u32(vad
[Intel-gfx] [PATCH] drm/i915: add quirk to enable backlight on Dell Chromebook 11 (2015)
Reported-by: Keith Webb
Suggested-by: Keith Webb
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=106671
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cfce7ea89d08..ec159df836a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14809,6 +14809,9 @@ static struct intel_quirk intel_quirks[] = {
/* Dell Chromebook 11 */
{ 0x0a06, 0x1028, 0x0a35, quirk_backlight_present },
+
+ /* Dell Chromebook 11 (2015 version) */
+ { 0x0a16, 0x1028, 0x0a35, quirk_backlight_present },
};
static void intel_init_quirks(struct drm_device *dev)
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Treat ringbuffer vaddress type properly when vmapped
Chris Wilson writes:
> On Fri, Oct 30, 2015 at 01:39:24PM +0200, Mika Kuoppala wrote:
>> commit def0c5f6b0cd ("drm/i915: Map the ringbuffer using WB on LLC machines")
>> enhanced ringbuffer access by vmapping the object instead of doing ioremap.
>>
>> The address space annotations however have been and should
>> remain to be __iomem, in order to get warnings if we
>> dereference the virtual addresses directly instead of using
>> proper accessors.
>>
>> To keep sparse happy, use explicit casts to __iomem and back
>> when we are wb capable.
>
> Or you could review the patch on list to remove the incorrect iomem.
Yes, ignore this patch.
> -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 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
On Fri, 30 Oct 2015, Ville Syrjälä wrote:
> On Fri, Oct 30, 2015 at 12:06:09PM +0200, Jani Nikula wrote:
>> On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
>> > From: Ville Syrjälä
>> >
>> > We get spurious PCH FIFO underruns if we enable the reporting too soon
>> > after enabling the crtc. Move it to be the last step, after the encoder
>> > enable. Additionally we need an extra vblank wait, otherwise we still
>> > get the underruns. Presumably the pipe/fdi isn't yet fully up and running
>> > otherwise.
>> >
>> > For symmetry, disable the PCH underrun reporting as the first thing,
>> > just before encoder disable, when shutting down the crtc.
>> >
>> > Signed-off-by: Ville Syrjälä
>> > ---
>> > drivers/gpu/drm/i915/intel_display.c | 13 +
>> > 1 file changed, 9 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 99fb33f..d5cb899 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc
>> > *crtc)
>> >intel_crtc->active = true;
>> >
>> >intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>> > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>> >
>> >for_each_encoder_on_crtc(dev, crtc, encoder)
>> >if (encoder->pre_enable)
>> > @@ -4912,6 +4911,12 @@ static void ironlake_crtc_enable(struct drm_crtc
>> > *crtc)
>> >
>> >if (HAS_PCH_CPT(dev))
>> >cpt_verify_modeset(dev, intel_crtc->pipe);
>> > +
>> > + if (intel_crtc->config->has_pch_encoder) {
>> > + /* Must wait for vblank to avoid spurious PCH FIFO underruns */
>> > + intel_wait_for_vblank(dev, pipe);
>> > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>>
>> Nitpick, moving this within the if (has_pch_encoder) isn't documented in
>> the commit message. Does that change have an impact?
>
> I don't much of a real concern here. I think the following might
> happen (all on the same pipe):
>
> 1. enable PCH port
> 2. disable PCH port
> 3. PCH FIFO underrun just after we've re-enabled the PCH
>underrun reporting
> 4. enable port A
> 5. PCH FIFO underrun reporting isn't enabled anymore for this pipe
>
> But since it's driving a non-PCH port anyway, that doesn't seem like
> a huge worry. But I suppose I could change it to always enable PCH
> FIFO underrun reporting even for port A. It should do no harm at least.
Mostly I just wanted this change documented in the commit message.
Jani.
>
>>
>> BR,
>> Jani.
>>
>> > + }
>> > }
>> >
>> > /* IPS only exists on ULT machines and is tied to pipe A. */
>> > @@ -5040,15 +5045,15 @@ static void ironlake_crtc_disable(struct drm_crtc
>> > *crtc)
>> >int pipe = intel_crtc->pipe;
>> >u32 reg, temp;
>> >
>> > + if (intel_crtc->config->has_pch_encoder)
>> > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>> > +
>> >for_each_encoder_on_crtc(dev, crtc, encoder)
>> >encoder->disable(encoder);
>> >
>> >drm_crtc_vblank_off(crtc);
>> >assert_vblank_disabled(crtc);
>> >
>> > - if (intel_crtc->config->has_pch_encoder)
>> > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
>> > -
>> >intel_disable_pipe(intel_crtc);
>> >
>> >ironlake_pfit_disable(intel_crtc, false);
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/14] drm/i915: Disable FIFO underrun reporting around IBX transcoder B workaround
On Fri, Oct 30, 2015 at 12:11:45PM +0200, Jani Nikula wrote:
> On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Doing the IBX transcoder B workaround causes underruns on
> > pipe/transcoder A. Just hide them by disabling underrun reporting for
> > pipe A around the workaround.
> >
> > It might be possible to avoid the underruns by moving the workaround
> > to be applied only when enabling pipe A. But I was too lazy to try it
> > right now, and the current method has been proven to work, so didn't
> > want to change it too hastily.
>
> Is it possible this enables underrun reporting on pipe A even if it
> wasn't enabled before?
Yes, it's possible. It would mean that pipe A is currently enabled, and
has already suffered an underrun (which is why the underrun reporting
got disabled). But I think that's OK. We would really want the underrun
reporting to rearm itself after a small delay anyway, but currently that
doesn't happen. I had a hacky patch for that at some point, but it would
probably need more work, and we should first fix up all the known bugs
that can cause underruns ie. watermark code.
>
> BR,
> Jani.
>
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 11 +++
> > drivers/gpu/drm/i915/intel_drv.h | 9 +
> > drivers/gpu/drm/i915/intel_hdmi.c | 11 +++
> > drivers/gpu/drm/i915/intel_sdvo.c | 11 +++
> > 4 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 8287df4..4a0fb63 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3957,6 +3957,13 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> > * matching HDMI port to be enabled on transcoder A.
> > */
> > if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) {
> > + /*
> > +* We get CPU/PCH FIFO underruns on the other pipe when
> > +* doing the workaround. Sweep them under the rug.
> > +*/
> > + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> > /* always enable with pattern 1 (as per spec) */
> > DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
> > DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
> > @@ -3966,6 +3973,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> > DP &= ~DP_PORT_EN;
> > I915_WRITE(intel_dp->output_reg, DP);
> > POSTING_READ(intel_dp->output_reg);
> > +
> > + intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> > + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> > }
> >
> > msleep(intel_dp->panel_power_down_delay);
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 72cc272..35f1457 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1073,6 +1073,15 @@ intel_wait_for_vblank(struct drm_device *dev, int
> > pipe)
> > {
> > drm_wait_one_vblank(dev, pipe);
> > }
> > +static inline void
> > +intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe)
> > +{
> > + const struct intel_crtc *crtc =
> > + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> > +
> > + if (crtc->active)
> > + intel_wait_for_vblank(dev, pipe);
> > +}
> > int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
> > void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
> > struct intel_digital_port *dport,
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 013bd7d..bccbe70 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1108,6 +1108,13 @@ static void intel_disable_hdmi(struct intel_encoder
> > *encoder)
> > * matching DP port to be enabled on transcoder A.
> > */
> > if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) {
> > + /*
> > +* We get CPU/PCH FIFO underruns on the other pipe when
> > +* doing the workaround. Sweep them under the rug.
> > +*/
> > + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> > +
> > temp &= ~SDVO_PIPE_B_SELECT;
> > temp |= SDVO_ENABLE;
> > /*
> > @@ -1122,6 +1129,10 @@ static void intel_disable_hdmi(struct intel_encoder
> > *encoder)
> > temp &= ~SDVO_ENABLE;
> > I915_WRITE(intel_hdmi->hdmi_reg, temp);
> > POSTING_READ(intel_hdmi->hdmi_reg);
> > +
> > + intel_wait_for_vblank_if_active(dev_priv->dev, P
[Intel-gfx] [drm-intel:topic/drm-misc 3/6] warning: (DRM_ATMEL_HLCDC && ..) selects DRM_KMS_CMA_HELPER which has unmet direct dependencies (HAS_IOMEM && ..)
tree: git://anongit.freedesktop.org/drm-intel topic/drm-misc head: 364b19868ecd11b91d83c921b55022f70c6c64c6 commit: f68b2f3ac407ca71bd0b22a88127087e0842862a [3/6] drm/imx: Remove local fbdev emulation Kconfig option config: m68k-allyesconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout f68b2f3ac407ca71bd0b22a88127087e0842862a # save the attached .config to linux build tree make.cross ARCH=m68k All warnings (new ones prefixed by >>): warning: (DRM_ATMEL_HLCDC && DRM_FSL_DCU && DRM_VC4) selects DRM_KMS_CMA_HELPER which has unmet direct dependencies (HAS_IOMEM && DRM && HAVE_DMA_ATTRS) --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
On Fri, Oct 30, 2015 at 12:06:09PM +0200, Jani Nikula wrote:
> On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > We get spurious PCH FIFO underruns if we enable the reporting too soon
> > after enabling the crtc. Move it to be the last step, after the encoder
> > enable. Additionally we need an extra vblank wait, otherwise we still
> > get the underruns. Presumably the pipe/fdi isn't yet fully up and running
> > otherwise.
> >
> > For symmetry, disable the PCH underrun reporting as the first thing,
> > just before encoder disable, when shutting down the crtc.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 13 +
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 99fb33f..d5cb899 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc
> > *crtc)
> > intel_crtc->active = true;
> >
> > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
> >
> > for_each_encoder_on_crtc(dev, crtc, encoder)
> > if (encoder->pre_enable)
> > @@ -4912,6 +4911,12 @@ static void ironlake_crtc_enable(struct drm_crtc
> > *crtc)
> >
> > if (HAS_PCH_CPT(dev))
> > cpt_verify_modeset(dev, intel_crtc->pipe);
> > +
> > + if (intel_crtc->config->has_pch_encoder) {
> > + /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> > + intel_wait_for_vblank(dev, pipe);
> > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>
> Nitpick, moving this within the if (has_pch_encoder) isn't documented in
> the commit message. Does that change have an impact?
I don't much of a real concern here. I think the following might
happen (all on the same pipe):
1. enable PCH port
2. disable PCH port
3. PCH FIFO underrun just after we've re-enabled the PCH
underrun reporting
4. enable port A
5. PCH FIFO underrun reporting isn't enabled anymore for this pipe
But since it's driving a non-PCH port anyway, that doesn't seem like
a huge worry. But I suppose I could change it to always enable PCH
FIFO underrun reporting even for port A. It should do no harm at least.
>
> BR,
> Jani.
>
> > + }
> > }
> >
> > /* IPS only exists on ULT machines and is tied to pipe A. */
> > @@ -5040,15 +5045,15 @@ static void ironlake_crtc_disable(struct drm_crtc
> > *crtc)
> > int pipe = intel_crtc->pipe;
> > u32 reg, temp;
> >
> > + if (intel_crtc->config->has_pch_encoder)
> > + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> > for_each_encoder_on_crtc(dev, crtc, encoder)
> > encoder->disable(encoder);
> >
> > drm_crtc_vblank_off(crtc);
> > assert_vblank_disabled(crtc);
> >
> > - if (intel_crtc->config->has_pch_encoder)
> > - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> > -
> > intel_disable_pipe(intel_crtc);
> >
> > ironlake_pfit_disable(intel_crtc, false);
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
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 i-g-t 2/3] Unify handling of slow/combinatorial tests
On Fri, Oct 30, 2015 at 09:55:03AM -0200, Paulo Zanoni wrote: > 2015-10-30 5:56 GMT-02:00 David Weinehall : > > On Wed, Oct 28, 2015 at 02:12:15PM -0200, Paulo Zanoni wrote: > >> 2015-10-28 9:29 GMT-02:00 David Weinehall > >> : > >> > Some tests should not be run by default, due to their slow, > >> > and sometimes superfluous, nature. > >> > > >> > We still want to be able to run these tests in some cases. > >> > Until now there's been no unified way of handling this. Remedy > >> > this by introducing the --all option to igt_core, > >> > and use it in gem_concurrent_blit & kms_frontbuffer_tracking. > >> > >> I really think you should explain both your plan and its > >> implementation in more details here. > > > > Well, I don't see how much more there is to explain; the idea is simply > > that different tests shouldn't implement similar behaviour in different > > manners (current kms_frontbuffer_tracking uses a command line option, > > gem_concurrent_blit changes behaviour depending on the file name it's > > called with). > > What made me write that is that I noticed that now --all is required > during --list-subtests (thanks for doing this!) but it was not easy to > notice this in the commit message or in the code. So I was thinking > something simple, such as a description of how to use the new option > both when running IGT and when writing subtests: > > "These tests will only appear in --list-subtests if you also specify > --all. Same for --run-subtest calls. There's this new macro > igt_subtest_slow_f (maybe igt_subtest_flags_f now?) which should be > used in case the subtest can be slow/combinatorial." > > > > >> > > >> > Signed-off-by: David Weinehall > >> > --- > >> > lib/igt_core.c | 24 + > >> > lib/igt_core.h | 7 ++ > >> > tests/gem_concurrent_blit.c | 44 - > >> > tests/kms_frontbuffer_tracking.c | 208 > >> > ++- > >> > 4 files changed, 165 insertions(+), 118 deletions(-) > >> > > >> > diff --git a/lib/igt_core.c b/lib/igt_core.c > >> > index 59127cafe606..6575b9d6bf0d 100644 > >> > --- a/lib/igt_core.c > >> > +++ b/lib/igt_core.c > >> > @@ -216,6 +216,7 @@ const char *igt_interactive_debug; > >> > > >> > /* subtests helpers */ > >> > static bool list_subtests = false; > >> > +static bool with_slow_combinatorial = false; > >> > >> The option is called --all, the new subtest macro is _slow and the > >> variables and enums are called with_slow_combinatorial. Is this > >> intentional? > > > > The option is called --all because "--with-slow-combinatorial" was > > considered to be too much of a mouthful. The variables & enums are > > still retaining these names because they're much more descriptive. > > Ok, let's keep is like this then (in case they don't change with > _flags suggestion). > > > > > The macro is called _slow because I wanted to keep it a bit shorter, > > but I can rename it to _slow_combinatorial if that's preferred. > > I agree _slow_combinatorial is too big, so let's keep it like this. > Maybe the new version is going to be _flags or something? igt_subtest_cond_f(). The subtest is included in the test lists if and only if the conditional experession is true. And run with igt_subtest_flags. -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 i-g-t 2/3] Unify handling of slow/combinatorial tests
2015-10-30 5:56 GMT-02:00 David Weinehall :
> On Wed, Oct 28, 2015 at 02:12:15PM -0200, Paulo Zanoni wrote:
>> 2015-10-28 9:29 GMT-02:00 David Weinehall :
>> > Some tests should not be run by default, due to their slow,
>> > and sometimes superfluous, nature.
>> >
>> > We still want to be able to run these tests in some cases.
>> > Until now there's been no unified way of handling this. Remedy
>> > this by introducing the --all option to igt_core,
>> > and use it in gem_concurrent_blit & kms_frontbuffer_tracking.
>>
>> I really think you should explain both your plan and its
>> implementation in more details here.
>
> Well, I don't see how much more there is to explain; the idea is simply
> that different tests shouldn't implement similar behaviour in different
> manners (current kms_frontbuffer_tracking uses a command line option,
> gem_concurrent_blit changes behaviour depending on the file name it's
> called with).
What made me write that is that I noticed that now --all is required
during --list-subtests (thanks for doing this!) but it was not easy to
notice this in the commit message or in the code. So I was thinking
something simple, such as a description of how to use the new option
both when running IGT and when writing subtests:
"These tests will only appear in --list-subtests if you also specify
--all. Same for --run-subtest calls. There's this new macro
igt_subtest_slow_f (maybe igt_subtest_flags_f now?) which should be
used in case the subtest can be slow/combinatorial."
>
>> >
>> > Signed-off-by: David Weinehall
>> > ---
>> > lib/igt_core.c | 24 +
>> > lib/igt_core.h | 7 ++
>> > tests/gem_concurrent_blit.c | 44 -
>> > tests/kms_frontbuffer_tracking.c | 208
>> > ++-
>> > 4 files changed, 165 insertions(+), 118 deletions(-)
>> >
>> > diff --git a/lib/igt_core.c b/lib/igt_core.c
>> > index 59127cafe606..6575b9d6bf0d 100644
>> > --- a/lib/igt_core.c
>> > +++ b/lib/igt_core.c
>> > @@ -216,6 +216,7 @@ const char *igt_interactive_debug;
>> >
>> > /* subtests helpers */
>> > static bool list_subtests = false;
>> > +static bool with_slow_combinatorial = false;
>>
>> The option is called --all, the new subtest macro is _slow and the
>> variables and enums are called with_slow_combinatorial. Is this
>> intentional?
>
> The option is called --all because "--with-slow-combinatorial" was
> considered to be too much of a mouthful. The variables & enums are
> still retaining these names because they're much more descriptive.
Ok, let's keep is like this then (in case they don't change with
_flags suggestion).
>
> The macro is called _slow because I wanted to keep it a bit shorter,
> but I can rename it to _slow_combinatorial if that's preferred.
I agree _slow_combinatorial is too big, so let's keep it like this.
Maybe the new version is going to be _flags or something?
>
>>
>> > static char *run_single_subtest = NULL;
>> > static bool run_single_subtest_found = false;
>> > static const char *in_subtest = NULL;
>> > @@ -235,6 +236,7 @@ bool test_child;
>> >
>> > enum {
>> > OPT_LIST_SUBTESTS,
>> > + OPT_WITH_SLOW_COMBINATORIAL,
>> > OPT_RUN_SUBTEST,
>> > OPT_DESCRIPTION,
>> > OPT_DEBUG,
>> > @@ -478,6 +480,7 @@ static void print_usage(const char *help_str, bool
>> > output_on_stderr)
>> >
>> > fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
>> > fprintf(f, " --list-subtests\n"
>> > + " --all\n"
>> >" --run-subtest \n"
>> >" --debug[=log-domain]\n"
>> >" --interactive-debug[=domain]\n"
>> > @@ -510,6 +513,7 @@ static int common_init(int *argc, char **argv,
>> > int c, option_index = 0, i, x;
>> > static struct option long_options[] = {
>> > {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
>> > + {"all", 0, 0, OPT_WITH_SLOW_COMBINATORIAL},
>> > {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
>> > {"help-description", 0, 0, OPT_DESCRIPTION},
>> > {"debug", optional_argument, 0, OPT_DEBUG},
>> > @@ -617,6 +621,10 @@ static int common_init(int *argc, char **argv,
>> > if (!run_single_subtest)
>> > list_subtests = true;
>> > break;
>> > + case OPT_WITH_SLOW_COMBINATORIAL:
>> > + if (!run_single_subtest)
>> > + with_slow_combinatorial = true;
>> > + break;
>> > case OPT_RUN_SUBTEST:
>> > if (!list_subtests)
>> > run_single_subtest = strdup(optarg);
>> > @@ -1629,6 +1637,22 @@ void igt_skip_on_simulation(void)
>> > igt_require(!igt_run_in_simulation());
>> > }
>> >
>> > +/**
>> > + * __igt_slow_combinatorial:
>> > + *
>> > + * This is used to skip subtests that should
Re: [Intel-gfx] [PATCH] drm/i915: Treat ringbuffer vaddress type properly when vmapped
On Fri, Oct 30, 2015 at 01:39:24PM +0200, Mika Kuoppala wrote:
> commit def0c5f6b0cd ("drm/i915: Map the ringbuffer using WB on LLC machines")
> enhanced ringbuffer access by vmapping the object instead of doing ioremap.
>
> The address space annotations however have been and should
> remain to be __iomem, in order to get warnings if we
> dereference the virtual addresses directly instead of using
> proper accessors.
>
> To keep sparse happy, use explicit casts to __iomem and back
> when we are wb capable.
Or you could review the patch on list to remove the incorrect iomem.
-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: Avoid pointer arithmetic in calculating plane surface offset
On 30/10/15 11:26, Mika Kuoppala wrote:
VMA offsets are 64 bits. Plane surface offsets are in ggtt and
the hardware register to set this is thus 32 bits. Be explicit
about these and convert carefully to from vma to final size.
This will make sparse happy by not creating 32bit pointers out
of 64bit vma offsets.
Cc: Tvrtko Ursulin
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_display.c | 16 +---
drivers/gpu/drm/i915/intel_drv.h | 6 +++---
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3f1b545..37e83f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2926,13 +2926,13 @@ u32 intel_fb_stride_alignment(struct drm_device *dev,
uint64_t fb_modifier,
}
}
-unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-struct drm_i915_gem_object *obj,
-unsigned int plane)
+u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
+ struct drm_i915_gem_object *obj,
+ unsigned int plane)
{
const struct i915_ggtt_view *view = &i915_ggtt_view_normal;
struct i915_vma *vma;
- unsigned char *offset;
+ u64 offset;
if (intel_rotation_90_or_270(intel_plane->base.state->rotation))
view = &i915_ggtt_view_rotated;
@@ -2942,14 +2942,16 @@ unsigned long intel_plane_obj_offset(struct intel_plane
*intel_plane,
view->type))
return -1;
- offset = (unsigned char *)vma->node.start;
+ offset = vma->node.start;
if (plane == 1) {
offset += vma->ggtt_view.rotation_info.uv_start_page *
PAGE_SIZE;
}
- return (unsigned long)offset;
+ WARN_ON(upper_32_bits(offset));
+
+ return lower_32_bits(offset);
}
static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
@@ -3075,7 +3077,7 @@ static void skylake_update_primary_plane(struct drm_crtc
*crtc,
u32 tile_height, plane_offset, plane_size;
unsigned int rotation;
int x_offset, y_offset;
- unsigned long surf_addr;
+ u32 surf_addr;
struct intel_crtc_state *crtc_state = intel_crtc->config;
struct intel_plane_state *plane_state;
int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a3bbdc..bc60385 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1196,9 +1196,9 @@ void intel_modeset_preclose(struct drm_device *dev,
struct drm_file *file);
int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state
*crtc_state);
-unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-struct drm_i915_gem_object *obj,
-unsigned int plane);
+u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
+ struct drm_i915_gem_object *obj,
+ unsigned int plane);
u32 skl_plane_ctl_format(uint32_t pixel_format);
u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 4276c13..a2c15f8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -194,7 +194,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct
drm_crtc *crtc,
u32 plane_ctl, stride_div, stride;
const struct drm_intel_sprite_colorkey *key =
&to_intel_plane_state(drm_plane->state)->ckey;
- unsigned long surf_addr;
+ u32 surf_addr;
u32 tile_height, plane_offset, plane_size;
unsigned int rotation;
int x_offset, y_offset;
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Treat ringbuffer vaddress type properly when vmapped
commit def0c5f6b0cd ("drm/i915: Map the ringbuffer using WB on LLC machines")
enhanced ringbuffer access by vmapping the object instead of doing ioremap.
The address space annotations however have been and should
remain to be __iomem, in order to get warnings if we
dereference the virtual addresses directly instead of using
proper accessors.
To keep sparse happy, use explicit casts to __iomem and back
when we are wb capable.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c9b081f..97b5654 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1997,14 +1997,14 @@ static int init_phys_status_page(struct intel_engine_cs
*ring)
void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
{
if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
- vunmap(ringbuf->virtual_start);
+ vunmap((void __force *)ringbuf->virtual_start);
else
iounmap(ringbuf->virtual_start);
ringbuf->virtual_start = NULL;
i915_gem_object_ggtt_unpin(ringbuf->obj);
}
-static u32 *vmap_obj(struct drm_i915_gem_object *obj)
+static void *vmap_obj(struct drm_i915_gem_object *obj)
{
struct sg_page_iter sg_iter;
struct page **pages;
@@ -2043,7 +2043,7 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device
*dev,
return ret;
}
- ringbuf->virtual_start = vmap_obj(obj);
+ ringbuf->virtual_start = (void __iomem *)vmap_obj(obj);
if (ringbuf->virtual_start == NULL) {
i915_gem_object_ggtt_unpin(obj);
return -ENOMEM;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 1/1] drm/i915/gen9: Check BIOS RC6 setup before enabling RC6
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
configuration registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.
v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
not enabled in case BIOS disabled RC6.
Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Signed-off-by: Sagar Arun Kamble
---
drivers/gpu/drm/i915/intel_pm.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2c7c9fc..1ec52f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -30,6 +30,7 @@
#include "intel_drv.h"
#include "../../../platform/x86/intel_ips.h"
#include
+#include
/**
* RC6 is a special power stage which allows the GPU to enter an very
@@ -4763,6 +4764,20 @@ static void gen9_enable_rc6(struct drm_device *dev)
struct intel_engine_cs *ring;
uint32_t rc6_mask = 0;
int unused;
+ bool hw_rc6_enabled, sw_rc6_enabled;
+ struct device *device = &dev->pdev->dev;
+
+ /* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
+ hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+ (GEN6_RC_CTL_RC6_ENABLE |
GEN6_RC_CTL_HW_ENABLE);
+ sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
+ && (I915_READ(GEN6_RC_STATE) & 0x4);
+ if (!(hw_rc6_enabled || sw_rc6_enabled)) {
+ i915.enable_rc6 = 0;
+ pm_runtime_forbid(device);
+ DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");
+ }
+
/* 1a: Software RC state - RC0 */
I915_WRITE(GEN6_RC_STATE, 0);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Avoid pointer arithmetic in calculating plane surface offset
VMA offsets are 64 bits. Plane surface offsets are in ggtt and
the hardware register to set this is thus 32 bits. Be explicit
about these and convert carefully to from vma to final size.
This will make sparse happy by not creating 32bit pointers out
of 64bit vma offsets.
Cc: Tvrtko Ursulin
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_display.c | 16 +---
drivers/gpu/drm/i915/intel_drv.h | 6 +++---
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3f1b545..37e83f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2926,13 +2926,13 @@ u32 intel_fb_stride_alignment(struct drm_device *dev,
uint64_t fb_modifier,
}
}
-unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-struct drm_i915_gem_object *obj,
-unsigned int plane)
+u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
+ struct drm_i915_gem_object *obj,
+ unsigned int plane)
{
const struct i915_ggtt_view *view = &i915_ggtt_view_normal;
struct i915_vma *vma;
- unsigned char *offset;
+ u64 offset;
if (intel_rotation_90_or_270(intel_plane->base.state->rotation))
view = &i915_ggtt_view_rotated;
@@ -2942,14 +2942,16 @@ unsigned long intel_plane_obj_offset(struct intel_plane
*intel_plane,
view->type))
return -1;
- offset = (unsigned char *)vma->node.start;
+ offset = vma->node.start;
if (plane == 1) {
offset += vma->ggtt_view.rotation_info.uv_start_page *
PAGE_SIZE;
}
- return (unsigned long)offset;
+ WARN_ON(upper_32_bits(offset));
+
+ return lower_32_bits(offset);
}
static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
@@ -3075,7 +3077,7 @@ static void skylake_update_primary_plane(struct drm_crtc
*crtc,
u32 tile_height, plane_offset, plane_size;
unsigned int rotation;
int x_offset, y_offset;
- unsigned long surf_addr;
+ u32 surf_addr;
struct intel_crtc_state *crtc_state = intel_crtc->config;
struct intel_plane_state *plane_state;
int src_x = 0, src_y = 0, src_w = 0, src_h = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a3bbdc..bc60385 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1196,9 +1196,9 @@ void intel_modeset_preclose(struct drm_device *dev,
struct drm_file *file);
int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state
*crtc_state);
-unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
-struct drm_i915_gem_object *obj,
-unsigned int plane);
+u32 intel_plane_obj_offset(struct intel_plane *intel_plane,
+ struct drm_i915_gem_object *obj,
+ unsigned int plane);
u32 skl_plane_ctl_format(uint32_t pixel_format);
u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 4276c13..a2c15f8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -194,7 +194,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct
drm_crtc *crtc,
u32 plane_ctl, stride_div, stride;
const struct drm_intel_sprite_colorkey *key =
&to_intel_plane_state(drm_plane->state)->ckey;
- unsigned long surf_addr;
+ u32 surf_addr;
u32 tile_height, plane_offset, plane_size;
unsigned int rotation;
int x_offset, y_offset;
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp: add eDP DPCD backlight control bit definitions
On Thu, Oct 29, 2015 at 02:27:32PM +0200, Ander Conselvan De Oliveira wrote: > On Thu, 2015-10-29 at 11:03 +0200, Jani Nikula wrote: > > Cc: Yetunde Adebisi > > Signed-off-by: Jani Nikula > > --- > > include/drm/drm_dp_helper.h | 36 > > 1 file changed, 36 insertions(+) > > > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > > index bb9d0deca07c..1252108da0ef 100644 > > --- a/include/drm/drm_dp_helper.h > > +++ b/include/drm/drm_dp_helper.h > > @@ -455,16 +455,52 @@ > > # define DP_EDP_14 0x03 > > > > #define DP_EDP_GENERAL_CAP_1 0x701 > > +# define DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP (1 << 0) > > +# define DP_EDP_BACKLIGHT_PIN_ENABLE_CAP (1 << 1) > > +# define DP_EDP_BACKLIGHT_AUX_ENABLE_CAP (1 << 2) > > +# define DP_EDP_PANEL_SELF_TEST_PIN_ENABLE_CAP (1 << 3) > > +# define DP_EDP_PANEL_SELF_TEST_AUX_ENABLE_CAP (1 << 4) > > +# define DP_EDP_FRC_ENABLE_CAP (1 << 5) > > +# define DP_EDP_COLOR_ENGINE_CAP (1 << 6) > > +# define DP_EDP_SET_POWER_CAP (1 << 7) > > > > #define DP_EDP_BACKLIGHT_ADJUSTMENT_CAP 0x702 > > +# define DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP (1 << 0) > > +# define DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP (1 << 1) > > +# define DP_EDP_BACKLIGHT_BRIGHTNESS_BYTE_COUNT(1 << 2) > > +# define DP_EDP_BACKLIGHT_AUX_PWM_PRODUCT_CAP (1 << 3) > > +# define DP_EDP_BACKLIGHT_FREQ_PWM_PIN_PASSTHRU_CAP(1 << 4) > > +# define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > > +# define DP_EDP_DYNAMIC_BACKLIGHT_CAP (1 << 6) > > +# define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP(1 << 7) > > > > #define DP_EDP_GENERAL_CAP_2 0x703 > > +# define DP_EDP_OVERDRIVE_ENGINE_ENABLED (1 << 0) > > > > #define DP_EDP_GENERAL_CAP_3 0x704/* eDP 1.4 */ > > +# define DP_EDP_X_REGION_CAP_MASK (0xf << 0) > > +# define DP_EDP_X_REGION_CAP_SHIFT 0 > > +# define DP_EDP_Y_REGION_CAP_MASK (0xf << 4) > > +# define DP_EDP_Y_REGION_CAP_SHIFT 4 > > > > #define DP_EDP_DISPLAY_CONTROL_REGISTER 0x720 > > +# define DP_EDP_BACKLIGHT_ENABLE (1 << 0) > > +# define DP_EDP_BLACK_VIDEO_ENABLE (1 << 1) > > +# define DP_EDP_FRC_ENABLE (1 << 2) > > +# define DP_EDP_COLOR_ENGINE_ENABLE(1 << 3) > > +# define DP_EDP_VBLANK_BACKLIGHT_UPDATE_ENABLE (1 << 7) > > > > #define DP_EDP_BACKLIGHT_MODE_SET_REGISTER 0x721 > > +# define DP_EDP_BACKLIGHT_CONTROL_MODE_MASK(3 << 0) > > +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PWM (0 << 0) > > +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET (1 << 0) > > +# define DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD(2 << 0) > > +# define DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT (3 << 0) > > +# define DP_EDP_BACKLIGHT_FREQ_PWM_PIN_PASSTHRU_ENABLE (1 << 2) > > +# define DP_EDP_BACKLIGHT_FREQ_AUX_SET_ENABLE (1 << 3) > > +# define DP_EDP_DYNAMIC_BACKLIGHT_ENABLE (1 << 4) > > +# define DP_EDP_REGIONAL_BACKLIGHT_ENABLE (1 << 5) > > Doesn't DP_EDP_REGIONAL_BACKLIGHT_ENABLE need a /* eDP 1.4 */ comment too? > > Anyway, it all matches the spec so, > > Reviewed-by: Ander Conselvan de Oliveira Applied to drm-misc, thanks. -Daniel > > > +# define DP_EDP_UPDATE_REGION_BRIGHTNESS (1 << 6) /* eDP 1.4 > > */ > > > > #define DP_EDP_BACKLIGHT_BRIGHTNESS_MSB 0x722 > > #define DP_EDP_BACKLIGHT_BRIGHTNESS_LSB 0x723 > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation 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] igt/kms_rotation_crc: Add a subtest to validate Y-tiled obj + Y fb modifier (v5)
On 30/10/15 01:44, Vivek Kasireddy wrote:
The main goal of this subtest is to trigger the following warning in
the function i915_gem_object_get_fence():
if (WARN_ON(!obj->map_and_fenceable))
To trigger this warning, the subtest first creates a Y-tiled object and
an associated framebuffer with the Y-fb modifier. Furthermore, to
prevent the map_and_fenceable from being set, we make sure that
the object does not have a normal VMA by refraining from rendering to the
object and by setting the rotation property upfront before calling commit.
v2: Do not call paint_squares and just use one output.
v3: Convert an if condition to igt_require and move the plane rotation
requirement further up before the fb allocation.
v4: After setting rotation to 90 and committing, change the rotation to
0 and commit once more. This is to test if the i915 driver hits any
warnings while pinning and unpinning an object that has both normal
and rotated views.
v5:
- Add another subtest to toggle the order of rotation
- Exhaustively test the i915 driver's pinning and unpinning code paths
for any fence leaks by iterating until MAX available fences.
Cc: Tvrtko Ursulin
Signed-off-by: Vivek Kasireddy
---
tests/kms_rotation_crc.c | 84
1 file changed, 84 insertions(+)
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index cc9847e..34f8150 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -264,6 +264,80 @@ static void test_plane_rotation(data_t *data, enum
igt_plane plane_type)
igt_require_f(valid_tests, "no valid crtc/connector combinations
found\n");
}
+static void test_plane_rotation_ytiled_obj(data_t *data, enum igt_plane
plane_type,
+ int toggle)
+{
+ igt_display_t *display = &data->display;
+ uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
+ uint32_t format = DRM_FORMAT_XRGB;
+ int bpp = igt_drm_format_to_bpp(format);
+ enum igt_commit_style commit = COMMIT_LEGACY;
+ int fd = data->gfx_fd;
+ igt_output_t *output = &display->outputs[0];
+ igt_plane_t *plane;
+ drmModeModeInfo *mode;
+ unsigned int stride, size, w, h;
+ uint32_t gem_handle;
+ int num_fences = gem_available_fences(fd);
+ int i, ret;
+
+ igt_require(output != NULL && output->valid == true);
+
+ plane = igt_output_get_plane(output, plane_type);
+ igt_require(igt_plane_supports_rotation(plane));
+
+ if (plane_type == IGT_PLANE_PRIMARY || plane_type == IGT_PLANE_CURSOR) {
+ igt_require(data->display.has_universal_planes);
+ commit = COMMIT_UNIVERSAL;
+ }
+
+ mode = igt_output_get_mode(output);
+ w = mode->hdisplay;
+ h = mode->vdisplay;
+
+ for (stride = 512; stride < (w * bpp / 8); stride *= 2)
+ ;
+ for (size = 1024*1024; size < stride * h; size *= 2)
+ ;
+
+ gem_handle = gem_create(fd, size);
+ ret = __gem_set_tiling(fd, gem_handle, I915_TILING_Y, stride);
+ igt_assert(ret == 0);
+
+ do_or_die(__kms_addfb(fd, gem_handle, w, h, stride,
+ format, tiling, LOCAL_DRM_MODE_FB_MODIFIERS,
+ &data->fb.fb_id));
+ data->fb.width = w;
+ data->fb.height = h;
+ data->fb.gem_handle = gem_handle;
+
+ igt_plane_set_fb(plane, NULL);
+ igt_display_commit(display);
+
+ igt_plane_set_fb(plane, &data->fb);
+
+ for (i = 0; i < num_fences + 1; i++) {
+ igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_0 :
IGT_ROTATION_90);
+ drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+DRM_MODE_OBJECT_PLANE,
+plane->rotation_property,
+plane->rotation);
+ ret = igt_display_try_commit2(display, commit);
+ igt_assert(ret == 0);
+
+ igt_plane_set_rotation(plane, toggle ? IGT_ROTATION_90 :
IGT_ROTATION_0);
+ drmModeObjectSetProperty(fd, plane->drm_plane->plane_id,
+DRM_MODE_OBJECT_PLANE,
+plane->rotation_property,
+plane->rotation);
+ ret = igt_display_try_commit2(display, commit);
+ igt_assert(ret == 0);
+ }
It manages to exhaust the fences with only one object? I was expecting
that multiple objects will be required since if it is only one how come
it allocates more than one fence register?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/14] drm/i915: Disable FIFO underrun reporting around IBX transcoder B workaround
On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Doing the IBX transcoder B workaround causes underruns on
> pipe/transcoder A. Just hide them by disabling underrun reporting for
> pipe A around the workaround.
>
> It might be possible to avoid the underruns by moving the workaround
> to be applied only when enabling pipe A. But I was too lazy to try it
> right now, and the current method has been proven to work, so didn't
> want to change it too hastily.
Is it possible this enables underrun reporting on pipe A even if it
wasn't enabled before?
BR,
Jani.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 11 +++
> drivers/gpu/drm/i915/intel_drv.h | 9 +
> drivers/gpu/drm/i915/intel_hdmi.c | 11 +++
> drivers/gpu/drm/i915/intel_sdvo.c | 11 +++
> 4 files changed, 42 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8287df4..4a0fb63 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3957,6 +3957,13 @@ intel_dp_link_down(struct intel_dp *intel_dp)
>* matching HDMI port to be enabled on transcoder A.
>*/
> if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) {
> + /*
> + * We get CPU/PCH FIFO underruns on the other pipe when
> + * doing the workaround. Sweep them under the rug.
> + */
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> +
> /* always enable with pattern 1 (as per spec) */
> DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
> DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
> @@ -3966,6 +3973,10 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> DP &= ~DP_PORT_EN;
> I915_WRITE(intel_dp->output_reg, DP);
> POSTING_READ(intel_dp->output_reg);
> +
> + intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> }
>
> msleep(intel_dp->panel_power_down_delay);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 72cc272..35f1457 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1073,6 +1073,15 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
> {
> drm_wait_one_vblank(dev, pipe);
> }
> +static inline void
> +intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe)
> +{
> + const struct intel_crtc *crtc =
> + to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +
> + if (crtc->active)
> + intel_wait_for_vblank(dev, pipe);
> +}
> int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
> void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
>struct intel_digital_port *dport,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 013bd7d..bccbe70 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1108,6 +1108,13 @@ static void intel_disable_hdmi(struct intel_encoder
> *encoder)
>* matching DP port to be enabled on transcoder A.
>*/
> if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) {
> + /*
> + * We get CPU/PCH FIFO underruns on the other pipe when
> + * doing the workaround. Sweep them under the rug.
> + */
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, false);
> +
> temp &= ~SDVO_PIPE_B_SELECT;
> temp |= SDVO_ENABLE;
> /*
> @@ -1122,6 +1129,10 @@ static void intel_disable_hdmi(struct intel_encoder
> *encoder)
> temp &= ~SDVO_ENABLE;
> I915_WRITE(intel_hdmi->hdmi_reg, temp);
> POSTING_READ(intel_hdmi->hdmi_reg);
> +
> + intel_wait_for_vblank_if_active(dev_priv->dev, PIPE_A);
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> + intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A, true);
> }
>
> intel_hdmi->set_infoframes(&encoder->base, false, NULL);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index c42b636..267e6cb 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1464,12 +1464,23 @@ static void intel_disable_sdvo(struct intel_encoder
> *encoder)
>* matching DP port to be enabled on transcoder A.
>*/
> if (HAS_PCH_IBX(dev_priv) && crtc->pipe == PIPE_B) {
>
Re: [Intel-gfx] [PATCH v6 1/2] drm/dp: Add a drm_aux-dev module for reading/writing dpcd registers.
On Thu, Oct 29, 2015 at 04:23:45PM -0700, Rafael Antognolli wrote:
> This module is heavily based on i2c-dev. Once loaded, it provides one
> dev node per DP AUX channel, named drm_dp_auxN, where N is an integer.
>
> It's possible to know which connector owns this aux channel by looking
> at the respective sysfs /sys/class/drm_aux_dev/drm_dp_auxN/connector, if
> the connector device pointer was correctly set in the aux helper struct.
>
> Two main operations are provided on the registers read and write. The
> address of the register to be read or written is given using lseek. The
> seek position is updated upon read or write.
Note that we (i915 folks at least) generally like to have the changelog
in the commit message itself. So maybe move it here for the final
version.
Anyways, this is looking rather nice now. I spotted a few remaining style
issues, and some error handling problems. Once those are dealt with
I think we should be done.
>
> Signed-off-by: Rafael Antognolli
> ---
> drivers/gpu/drm/Kconfig | 8 +
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_dp_aux_dev.c | 381
> +++
> drivers/gpu/drm/drm_dp_helper.c | 5 +
> include/drm/drm_dp_aux_dev.h | 50 +
> 5 files changed, 445 insertions(+)
> create mode 100644 drivers/gpu/drm/drm_dp_aux_dev.c
> create mode 100644 include/drm/drm_dp_aux_dev.h
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index c4bf9a1..daefcce 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -25,6 +25,14 @@ config DRM_MIPI_DSI
> bool
> depends on DRM
>
> +config DRM_DP_AUX_CHARDEV
> + bool "DRM DP AUX Interface"
> + depends on DRM
> + help
> + Choose this option to enable a /dev/drm_dp_auxN node that allows to
> + read and write values to arbitrary DPCD registers on the DP aux
> + channel.
> +
> config DRM_KMS_HELPER
> tristate
> depends on DRM
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1e9ff4c..e48ec8f 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -28,6 +28,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o
> drm_probe_helper.o \
> drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
> drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
> +drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
>
> obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>
> diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> b/drivers/gpu/drm/drm_dp_aux_dev.c
> new file mode 100644
> index 000..16dbc2e
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> @@ -0,0 +1,381 @@
> +/*
> + * Copyright © 2015 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:
> + *Rafael Antognolli
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct drm_dp_aux_dev {
> + unsigned index;
> + struct drm_dp_aux *aux;
> + struct device *dev;
> + struct kref refcount;
> + atomic_t usecount;
> +};
> +
> +#define DRM_AUX_MINORS 256
> +#define AUX_MAX_OFFSET (1 << 20)
> +static DEFINE_IDR(aux_idr);
> +static DEFINE_MUTEX(aux_idr_mutex);
> +static struct class *drm_dp_aux_dev_class;
> +static int drm_dev_major = -1;
> +
> +static struct drm_dp_aux_dev *drm_dp_aux_dev_get_by_minor(unsigned index)
> +{
> + struct drm_dp_aux_dev *aux_dev = NULL;
> +
> + mutex_lock(&aux_idr_mutex);
> + aux_dev = idr_find(&aux_idr, index);
> + if (!kref_get_unless_zero(&aux_dev->refcount))
> + aux_dev = NULL;
> + mutex_unlock(&aux_idr_mutex);
> +
> + return aux_dev;
> +}
Re: [Intel-gfx] [PATCH 03/14] drm/i915: Enable PCH FIFO underruns later on ILK/SNB/IVB
On Thu, 29 Oct 2015, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> We get spurious PCH FIFO underruns if we enable the reporting too soon
> after enabling the crtc. Move it to be the last step, after the encoder
> enable. Additionally we need an extra vblank wait, otherwise we still
> get the underruns. Presumably the pipe/fdi isn't yet fully up and running
> otherwise.
>
> For symmetry, disable the PCH underrun reporting as the first thing,
> just before encoder disable, when shutting down the crtc.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 99fb33f..d5cb899 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4874,7 +4874,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> intel_crtc->active = true;
>
> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
>
> for_each_encoder_on_crtc(dev, crtc, encoder)
> if (encoder->pre_enable)
> @@ -4912,6 +4911,12 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>
> if (HAS_PCH_CPT(dev))
> cpt_verify_modeset(dev, intel_crtc->pipe);
> +
> + if (intel_crtc->config->has_pch_encoder) {
> + /* Must wait for vblank to avoid spurious PCH FIFO underruns */
> + intel_wait_for_vblank(dev, pipe);
> + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
Nitpick, moving this within the if (has_pch_encoder) isn't documented in
the commit message. Does that change have an impact?
BR,
Jani.
> + }
> }
>
> /* IPS only exists on ULT machines and is tied to pipe A. */
> @@ -5040,15 +5045,15 @@ static void ironlake_crtc_disable(struct drm_crtc
> *crtc)
> int pipe = intel_crtc->pipe;
> u32 reg, temp;
>
> + if (intel_crtc->config->has_pch_encoder)
> + intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> +
> for_each_encoder_on_crtc(dev, crtc, encoder)
> encoder->disable(encoder);
>
> drm_crtc_vblank_off(crtc);
> assert_vblank_disabled(crtc);
>
> - if (intel_crtc->config->has_pch_encoder)
> - intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, false);
> -
> intel_disable_pipe(intel_crtc);
>
> ironlake_pfit_disable(intel_crtc, false);
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] FW: [PATCH v7 08/25] drm: Add color correction state flag
On Thu, 29 Oct 2015, "Sharma, Shashank" wrote: > HI Jani, > I am getting this warning, from kbuild, for the new flags being added in the > patch series in CRTC state. >>> include/drm/drm_crtc.h:314: warning: No description found for parameter >>> 'color_correction_changed' > > Can you please help me out by giving some details about how can I > resolve these ? You're adding a new field to a struct that has kernel-doc, but you're not adding kernel-doc for the new field. BR, Jani. > > Regards > Shashank > -Original Message- > From: lkp > Sent: Tuesday, October 20, 2015 7:29 PM > To: Sharma, Shashank > Cc: kbuild-...@01.org; dri-de...@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org; emil.l.veli...@gmail.com; Roper, Matthew D; > Bradford, Robert; Bish, Jim; Matheson, Annie J; kausalmall...@gmail.com; > Vetter, Daniel > Subject: Re: [Intel-gfx] [PATCH v7 08/25] drm: Add color correction state flag > > Hi Shashank, > > [auto build test WARNING on drm-intel/for-linux-next -- if it's inappropriate > base, please suggest rules for selecting the more suitable base] > > url: > https://github.com/0day-ci/linux/commits/Shashank-Sharma/Color-Management-for-DRM-framework/20151020-202959 > reproduce: make htmldocs > > All warnings (new ones prefixed by >>): > >drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for > parameter 'wedged' >drivers/gpu/drm/i915/i915_irq.c:2584: warning: No description found for > parameter 'fmt' >drivers/gpu/drm/drm_atomic.c:407: warning: No description found for > parameter 'dev' >>> include/drm/drm_crtc.h:314: warning: No description found for parameter >>> 'color_correction_changed' >include/drm/drm_crtc.h:314: warning: No description found for parameter > 'mode_blob' >include/drm/drm_crtc.h:314: warning: No description found for parameter > 'palette_before_ctm_blob' >include/drm/drm_crtc.h:314: warning: No description found for parameter > 'palette_after_ctm_blob' >include/drm/drm_crtc.h:314: warning: No description found for parameter > 'ctm_blob' >include/drm/drm_crtc.h:746: warning: No description found for parameter > 'tile_blob_ptr' >include/drm/drm_crtc.h:785: warning: No description found for parameter > 'rotation' >include/drm/drm_crtc.h:881: warning: No description found for parameter > 'mutex' >include/drm/drm_crtc.h:881: warning: No description found for parameter > 'helper_private' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'tile_idr' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'delayed_event' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'edid_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'dpms_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'path_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'tile_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'plane_type_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'rotation_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_src_x' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_src_y' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_src_w' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_src_h' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_crtc_x' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_crtc_y' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_crtc_w' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_crtc_h' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_fb_id' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_crtc_id' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_active' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'prop_mode_id' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'dvi_i_subconnector_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'dvi_i_select_subconnector_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'tv_subconnector_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'tv_select_subconnector_property' >include/drm/drm_crtc.h:1175: warning: No description found for parameter > 'tv_mode_property' >include/drm/drm_crtc.h:1175: warning: No description found for para
Re: [Intel-gfx] [PATCH] drm/i915: Skip fence installation for objects with rotated views (v4)
On Thu, Oct 29, 2015 at 06:54:38PM -0700, Vivek Kasireddy wrote:
> While pinning a fb object to the display plane, only install a fence
> if the object is using a normal view. This corresponds with the
> behavior found in i915_gem_object_do_pin() where the fencability
> criteria is determined only for objects with normal views.
>
> v2:
> Look at the object's map_and_fenceable flag to determine whether to
> install a fence or not (Chris).
>
> v3:
> Pin and unpin a fence only if the current view type is normal.
>
> v4:
> Extend the "view type is normal" check for pin_fence as well.
>
> Cc: Chris Wilson
> Cc: Tvrtko Ursulin
> Cc: Ville Syrjala
> Signed-off-by: Vivek Kasireddy
lgtm
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_display.c | 36
>
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 2fdfca1..9c80968 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2419,22 +2419,24 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>* framebuffer compression. For simplicity, we always install
>* a fence as the cost is not that onerous.
>*/
> - ret = i915_gem_object_get_fence(obj);
> - if (ret == -EDEADLK) {
> - /*
> - * -EDEADLK means there are no free fences
> - * no pending flips.
> - *
> - * This is propagated to atomic, but it uses
> - * -EDEADLK to force a locking recovery, so
> - * change the returned error to -EBUSY.
> - */
> - ret = -EBUSY;
> - goto err_unpin;
> - } else if (ret)
> - goto err_unpin;
> + if (view.type == I915_GGTT_VIEW_NORMAL) {
> + ret = i915_gem_object_get_fence(obj);
> + if (ret == -EDEADLK) {
> + /*
> + * -EDEADLK means there are no free fences
> + * no pending flips.
> + *
> + * This is propagated to atomic, but it uses
> + * -EDEADLK to force a locking recovery, so
> + * change the returned error to -EBUSY.
> + */
> + ret = -EBUSY;
> + goto err_unpin;
> + } else if (ret)
> + goto err_unpin;
>
> - i915_gem_object_pin_fence(obj);
> + i915_gem_object_pin_fence(obj);
> + }
>
> dev_priv->mm.interruptible = true;
> intel_runtime_pm_put(dev_priv);
> @@ -2460,7 +2462,9 @@ static void intel_unpin_fb_obj(struct drm_framebuffer
> *fb,
> ret = intel_fill_fb_ggtt_view(&view, fb, plane_state);
> WARN_ONCE(ret, "Couldn't get view from plane state!");
>
> - i915_gem_object_unpin_fence(obj);
> + if (view.type == I915_GGTT_VIEW_NORMAL)
> + i915_gem_object_unpin_fence(obj);
> +
> i915_gem_object_unpin_from_display_plane(obj, &view);
> }
>
> --
> 2.4.3
--
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] 4.0.8->4.1.3 : after resume from s2ram both internal and external display of a docked ThinkPad ate black
On 10/29/2015 10:49 PM, Pavel Machek wrote: > On Sun 2015-10-04 18:30:14, Toralf Förster wrote: >> On 08/04/2015 02:29 PM, Toralf Förster wrote: >>> On 08/02/2015 09:43 AM, Pavel Machek wrote: Any chance to bisect it? >>> Did it. >>> >>> FWIW: the mentioned commit was introduced between 3.18 and 3.19. >>> But my system (hardened 64 bit Gentoo) did not suffer from it till version >>> 4.0.8. >>> The hardened kernel 4.1.x was the first where the bug was visible at my >>> docked environment too. >>> >>> >>> >>> commit e7d6f7d708290da1b7c92f533444b042c79412e0 >>> Author: Dave Airlie >>> Date: Mon Dec 8 13:23:37 2014 +1000 >>> >>> drm/i915: resume MST after reading back hw state >>> >>> Otherwise the MST resume paths can hit DPMS paths >>> which hit state checker paths, which hit WARN_ON, >>> because the state checker is inconsistent with the >>> hw. >>> >>> This fixes a bunch of WARN_ON's on resume after >>> undocking. >>> >>> Signed-off-by: Dave Airlie >>> Reviewed-by: Daniel Vetter >>> Cc: sta...@vger.kernel.org >>> Signed-off-by: Jani Nikula >>> >> >> Is there anything else what I can do ? >> >> Current kernels up to 4.2.3 and 4.3-rc3 (not hardened) shows this issue here >> at my system. > > Yes. Now you ask Dave Airlie to fix it. If that Dear Dave, please fix it. Here's a work around which works for me since kernel 4.1.x : diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ab64d68..3aeead2 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -740,6 +740,8 @@ static int i915_drm_resume(struct drm_device *dev) if (dev_priv->display.hpd_irq_setup) dev_priv->display.hpd_irq_setup(dev); spin_unlock_irq(&dev_priv->irq_lock); + + intel_dp_mst_resume(dev); drm_modeset_lock_all(dev); intel_display_resume(dev); > does not work, you ask him to fix it, in less polite words. If that > does not work, you verify that reverting > e7d6f7d708290da1b7c92f533444b042c79412e0 fixes it for you, then ask > Daniel Vetter and Jani Nikula to revert it. If they fail to do that, > you go all the way up to Linus. > > Good luck ;-), > Pavel > -- Toralf, pgp key: C4EACDDE 0076E94E ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests
On Wed, Oct 28, 2015 at 02:12:15PM -0200, Paulo Zanoni wrote:
> 2015-10-28 9:29 GMT-02:00 David Weinehall :
> > Some tests should not be run by default, due to their slow,
> > and sometimes superfluous, nature.
> >
> > We still want to be able to run these tests in some cases.
> > Until now there's been no unified way of handling this. Remedy
> > this by introducing the --all option to igt_core,
> > and use it in gem_concurrent_blit & kms_frontbuffer_tracking.
>
> I really think you should explain both your plan and its
> implementation in more details here.
Well, I don't see how much more there is to explain; the idea is simply
that different tests shouldn't implement similar behaviour in different
manners (current kms_frontbuffer_tracking uses a command line option,
gem_concurrent_blit changes behaviour depending on the file name it's
called with).
> >
> > Signed-off-by: David Weinehall
> > ---
> > lib/igt_core.c | 24 +
> > lib/igt_core.h | 7 ++
> > tests/gem_concurrent_blit.c | 44 -
> > tests/kms_frontbuffer_tracking.c | 208
> > ++-
> > 4 files changed, 165 insertions(+), 118 deletions(-)
> >
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 59127cafe606..6575b9d6bf0d 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -216,6 +216,7 @@ const char *igt_interactive_debug;
> >
> > /* subtests helpers */
> > static bool list_subtests = false;
> > +static bool with_slow_combinatorial = false;
>
> The option is called --all, the new subtest macro is _slow and the
> variables and enums are called with_slow_combinatorial. Is this
> intentional?
The option is called --all because "--with-slow-combinatorial" was
considered to be too much of a mouthful. The variables & enums are
still retaining these names because they're much more descriptive.
The macro is called _slow because I wanted to keep it a bit shorter,
but I can rename it to _slow_combinatorial if that's preferred.
>
> > static char *run_single_subtest = NULL;
> > static bool run_single_subtest_found = false;
> > static const char *in_subtest = NULL;
> > @@ -235,6 +236,7 @@ bool test_child;
> >
> > enum {
> > OPT_LIST_SUBTESTS,
> > + OPT_WITH_SLOW_COMBINATORIAL,
> > OPT_RUN_SUBTEST,
> > OPT_DESCRIPTION,
> > OPT_DEBUG,
> > @@ -478,6 +480,7 @@ static void print_usage(const char *help_str, bool
> > output_on_stderr)
> >
> > fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
> > fprintf(f, " --list-subtests\n"
> > + " --all\n"
> >" --run-subtest \n"
> >" --debug[=log-domain]\n"
> >" --interactive-debug[=domain]\n"
> > @@ -510,6 +513,7 @@ static int common_init(int *argc, char **argv,
> > int c, option_index = 0, i, x;
> > static struct option long_options[] = {
> > {"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
> > + {"all", 0, 0, OPT_WITH_SLOW_COMBINATORIAL},
> > {"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> > {"help-description", 0, 0, OPT_DESCRIPTION},
> > {"debug", optional_argument, 0, OPT_DEBUG},
> > @@ -617,6 +621,10 @@ static int common_init(int *argc, char **argv,
> > if (!run_single_subtest)
> > list_subtests = true;
> > break;
> > + case OPT_WITH_SLOW_COMBINATORIAL:
> > + if (!run_single_subtest)
> > + with_slow_combinatorial = true;
> > + break;
> > case OPT_RUN_SUBTEST:
> > if (!list_subtests)
> > run_single_subtest = strdup(optarg);
> > @@ -1629,6 +1637,22 @@ void igt_skip_on_simulation(void)
> > igt_require(!igt_run_in_simulation());
> > }
> >
> > +/**
> > + * __igt_slow_combinatorial:
> > + *
> > + * This is used to skip subtests that should only be included
> > + * when the "--all" command line option has been specified. This version
> > + * is intended as a test.
> > + *
> > + * @slow_test: true if the subtest is part of the slow/combinatorial set
> > + *
> > + * Returns: true if the test should be run, false if the test should be
> > skipped
> > + */
> > +bool __igt_slow_combinatorial(bool slow_test)
> > +{
> > + return !slow_test || with_slow_combinatorial;
> > +}
> > +
> > /* structured logging */
> >
> > /**
> > diff --git a/lib/igt_core.h b/lib/igt_core.h
> > index 5ae09653fd55..7b592278bf6c 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -191,6 +191,12 @@ bool __igt_run_subtest(const char *subtest_name);
> > #define igt_subtest_f(f...) \
> > __igt_subtest_f(igt_tokencat(__tmpchar, __LINE__), f)
> >
> > +bool __igt_slow_combinatorial(bool slow_test);
> > +
>
> We also need a igt_subtest_slow() version (without "_f") and some
Re: [Intel-gfx] [PATCH i-g-t 2/3] Unify handling of slow/combinatorial tests
On Wed, Oct 28, 2015 at 05:14:28PM +, Thomas Wood wrote: > If this is intended to be documented and used in tests, then it should > be included in the public API (i.e. without the underscore prefix). True. Will fix. > > + * > > + * This is used to skip subtests that should only be included > > + * when the "--all" command line option has been specified. This version > > + * is intended as a test. > > + * > > + * @slow_test: true if the subtest is part of the slow/combinatorial set > > If this is used to test if a slow subtest should be run, shouldn't > slow_test always be true? The test is written such that igt_subtest_slow_f() can always be used both for fast and slow cases -- the slow flag will decide whether or not it should bail out (combined with the --all flag, obviously). So slow_test isn't always true. > Documentation for igt_subtest_slow_f is needed here. If __slow is > false, this macro just defines a normal subtest, which is > contradictory to its name. Perhaps igt_subtest_with_flags_f (or > similar) would be better and would also allow for future expansion > with other categories. Yeah, that could be a workable solution; in discussion with Daniel earlier we agreed not to do a "flags" implementation, to keep things simple for now, but it might indeed be better to do things right from the start. As we get to the point where we call igt_subtest with several different flags things will probably get quite complex though; at that point I suspect that the macro might start looking really hairy... Kind regards, David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
