[Intel-gfx] [PATCH] kms_rotation_crc: Add test for cursor rotation
From: Sonika Jindal
Signed-off-by: Sonika Jindal
---
tests/kms_rotation_crc.c | 80 ++
1 file changed, 59 insertions(+), 21 deletions(-)
diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
index 9f272fa..6c9674e 100644
--- a/tests/kms_rotation_crc.c
+++ b/tests/kms_rotation_crc.c
@@ -33,33 +33,49 @@ typedef struct {
int gfx_fd;
igt_display_t display;
struct igt_fb fb;
+ struct igt_fb fb_cursor;
igt_crc_t ref_crc;
igt_pipe_crc_t *pipe_crc;
} data_t;
static void
paint_squares(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
- igt_rotation_t rotation)
+ igt_rotation_t rotation, igt_plane_t *plane)
{
cairo_t *cr;
int w, h;
+ if (plane->is_cursor) {
+ w = 128;
+ h = 128;
+ cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb_cursor);
+
+ if (rotation == IGT_ROTATION_180) {
+ cairo_translate(cr, w, h);
+ cairo_rotate(cr, M_PI);
+ }
- w = mode->hdisplay;
- h = mode->vdisplay;
+ igt_paint_color(cr, 0, 0, w / 2, h / 2, .75, 0.5, 0.5);
+ igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.5, .75, 0.5);
+ igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.5, 0.5, .75);
+ igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, .75, .75, .75);
- cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+ } else {
+ w = mode->hdisplay;
+ h = mode->vdisplay;
- if (rotation == IGT_ROTATION_180) {
- cairo_translate(cr, w, h);
- cairo_rotate(cr, M_PI);
- }
+ cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
- /* Paint with 4 squares of Red, Green, White, Blue Clockwise */
- igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0);
- igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0);
- igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0);
- igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0);
+ if (rotation == IGT_ROTATION_180) {
+ cairo_translate(cr, w, h);
+ cairo_rotate(cr, M_PI);
+ }
+ /* Paint with 4 squares of Red, Green, White, Blue Clockwise */
+ igt_paint_color(cr, 0, 0, w / 2, h / 2, 1.0, 0.0, 0.0);
+ igt_paint_color(cr, w / 2, 0, w / 2, h / 2, 0.0, 1.0, 0.0);
+ igt_paint_color(cr, 0, h / 2, w / 2, h / 2, 0.0, 0.0, 1.0);
+ igt_paint_color(cr, w / 2, h / 2, w / 2, h / 2, 1.0, 1.0, 1.0);
+ }
cairo_destroy(cr);
}
@@ -68,7 +84,7 @@ static bool prepare_crtc(data_t *data, igt_output_t *output,
enum pipe pipe,
{
drmModeModeInfo *mode;
igt_display_t *display = &data->display;
- int fb_id;
+ int fb_id, fb_cursor_id;
igt_output_set_pipe(output, pipe);
@@ -91,9 +107,16 @@ static bool prepare_crtc(data_t *data, igt_output_t
*output, enum pipe pipe,
&data->fb);
igt_assert(fb_id);
+ fb_cursor_id = igt_create_fb(data->gfx_fd,
+ 128, 128,
+ DRM_FORMAT_ARGB,
+ false, /* tiled */
+ &data->fb_cursor);
+ igt_assert(fb_cursor_id);
+
+
/* Step 1: create a reference CRC for a software-rotated fb */
- paint_squares(data, &data->fb, mode, IGT_ROTATION_180);
/*
* XXX: We always set the primary plane to actually enable the pipe as
@@ -104,21 +127,32 @@ static bool prepare_crtc(data_t *data, igt_output_t
*output, enum pipe pipe,
igt_plane_t *primary;
primary = igt_output_get_plane(output, IGT_PLANE_PRIMARY);
+ paint_squares(data, &data->fb, mode, IGT_ROTATION_180, primary);
igt_plane_set_fb(primary, &data->fb);
}
- igt_plane_set_fb(plane, &data->fb);
+ if (plane->is_cursor) {
+ paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_180,
plane);
+ igt_plane_set_fb(plane, &data->fb_cursor);
+ } else {
+ paint_squares(data, &data->fb, mode, IGT_ROTATION_180, plane);
+ igt_plane_set_fb(plane, &data->fb);
+ }
igt_display_commit(display);
-
igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
/*
* Step 2: prepare the plane with an non-rotated fb let the hw
* rotate it.
*/
- paint_squares(data, &data->fb, mode, IGT_ROTATION_0);
+ if (plane->is_cursor) {
+ paint_squares(data, &data->fb_cursor, mode, IGT_ROTATION_0,
plane);
+ igt_plane_set_fb(plane, &data->fb_cursor);
+ } else {
+ paint_squares(data, &data->fb, mode, IGT_ROTATION_0, plane);
+
[Intel-gfx] [PATCH 2/2] drm/i915: Add rotation support for cursor plane
From: Sonika Jindal
The cursor plane also supports 180 degree rotation. Add a new
"cursor-rotation" property on the crtc which controls this.
Unlike sprites, the cursor has a fixed size, so if you have a small
cursor image with the rest of the bo filled by transparent pixels,
simply flipping the rotation property will cause the visible part
of the cursor to shift. This is something to keep in mind when
using cursor rotation.
v2: Fix gen4/vlv by offsetting the base address appropriately
v3: Removing cursor-rotation property and using rotation property on cursor
plane.
Testcase: kms_rotation_crc
Cc: Sagar Kamble
Signed-off-by: Ville Syrjälä
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_display.c | 26 +-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 15c0eaa..5a9fab9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4162,6 +4162,7 @@ enum punit_power_well {
#define MCURSOR_PIPE_A 0x00
#define MCURSOR_PIPE_B (1 << 28)
#define MCURSOR_GAMMA_ENABLE (1 << 26)
+#define CURSOR_ROTATE_180(1<<15)
#define CURSOR_TRICKLE_FEED_DISABLE (1 << 14)
#define _CURABASE 0x70084
#define _CURAPOS 0x70088
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 122ac6e..8c83bcc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8247,6 +8247,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32
base)
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
+ if (to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180))
+ cntl |= CURSOR_ROTATE_180;
+
if (intel_crtc->cursor_cntl != cntl) {
I915_WRITE(CURCNTR(pipe), cntl);
POSTING_READ(CURCNTR(pipe));
@@ -8302,6 +8305,13 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
I915_WRITE(CURPOS(pipe), pos);
+ /* ILK+ do this automagically */
+ if (HAS_GMCH_DISPLAY(dev) &&
+ to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) {
+ base += (intel_crtc->cursor_height *
+ intel_crtc->cursor_width - 1) * 4;
+ }
+
if (IS_845G(dev) || IS_I865G(dev))
i845_update_cursor(crtc, base);
else
@@ -8453,7 +8463,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
mutex_unlock(&dev->struct_mutex);
old_width = intel_crtc->cursor_width;
-
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
intel_crtc->cursor_width = width;
@@ -12074,6 +12083,7 @@ static const struct drm_plane_funcs
intel_cursor_plane_funcs = {
.update_plane = intel_cursor_plane_update,
.disable_plane = intel_cursor_plane_disable,
.destroy = intel_plane_destroy,
+ .set_property = intel_plane_set_property,
};
static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -12089,12 +12099,26 @@ static struct drm_plane
*intel_cursor_plane_create(struct drm_device *dev,
cursor->max_downscale = 1;
cursor->pipe = pipe;
cursor->plane = pipe;
+ cursor->rotation = BIT(DRM_ROTATE_0);
drm_universal_plane_init(dev, &cursor->base, 0,
&intel_cursor_plane_funcs,
intel_cursor_formats,
ARRAY_SIZE(intel_cursor_formats),
DRM_PLANE_TYPE_CURSOR);
+
+ if (INTEL_INFO(dev)->gen >= 4) {
+ if (!dev->mode_config.rotation_property)
+ dev->mode_config.rotation_property =
+ drm_mode_create_rotation_property(dev,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+ if (dev->mode_config.rotation_property)
+ drm_object_attach_property(&cursor->base.base,
+ dev->mode_config.rotation_property,
+ cursor->rotation);
+ }
+
return &cursor->base;
}
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Update plane parameters for cursor plane
From: Sonika Jindal
This allows the cursor plane to be updated the same way as primary and sprites,
and same set_property handler is used for all of these planes.
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_display.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 842a5e1..122ac6e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12019,6 +12019,22 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
};
+ const struct {
+ int crtc_x, crtc_y;
+ unsigned int crtc_w, crtc_h;
+ uint32_t src_x, src_y, src_w, src_h;
+ } orig = {
+ .crtc_x = crtc_x,
+ .crtc_y = crtc_y,
+ .crtc_w = crtc_w,
+ .crtc_h = crtc_h,
+ .src_x = src_x,
+ .src_y = src_y,
+ .src_w = src_w,
+ .src_h = src_h,
+ };
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+
bool visible;
int ret;
@@ -12032,6 +12048,17 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
crtc->cursor_x = crtc_x;
crtc->cursor_y = crtc_y;
+
+ intel_plane->crtc_x = orig.crtc_x;
+ intel_plane->crtc_y = orig.crtc_y;
+ intel_plane->crtc_w = orig.crtc_w;
+ intel_plane->crtc_h = orig.crtc_h;
+ intel_plane->src_x = orig.src_x;
+ intel_plane->src_y = orig.src_y;
+ intel_plane->src_w = orig.src_w;
+ intel_plane->src_h = orig.src_h;
+ intel_plane->obj = obj;
+
if (fb != crtc->cursor->fb) {
return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
} else {
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Don't enable IPS when pixel rate exceeds 95% of cdclk
On Fri, 12 Sep 2014, Ville Syrjälä wrote:
> On Fri, Sep 12, 2014 at 04:42:33PM +0100, Chris Wilson wrote:
>> On Fri, Sep 12, 2014 at 05:01:57PM +0300, ville.syrj...@linux.intel.com
>> wrote:
>> > From: Ville Syrjälä
>> >
>> > Bspec says we shouldn't enable IPS on BDW when the pipe pixel rate
>> > exceeds 95% of the core display clock. Apparently this can cause
>> > underruns.
>> >
>> > There's no similar restriction listed for HSW, so leave that one alone
>> > for now.
>> >
>> > v2: Add pipe_config_supports_ips() (Chris)
>> >
>> > Tested-by: Timo Aaltonen
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83497
>> > Signed-off-by: Ville Syrjälä
>> > ---
>> > drivers/gpu/drm/i915/intel_display.c | 21 +++--
>> > drivers/gpu/drm/i915/intel_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_pm.c | 16 +++-
>> > 3 files changed, 27 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 965eb3c..7809177 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -5241,12 +5241,29 @@ retry:
>> >return setup_ok ? 0 : -EINVAL;
>> > }
>> >
>> > +static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv,
>> > + struct intel_crtc_config *pipe_config)
>> > +{
>> > + if (pipe_config->pipe_bpp > 24)
>> > + return false;
>> > +
>> > + /* HSW can handle pixel rate up to cdclk? */
>> > + if (IS_HASWELL(dev_priv->dev))
>>
>> This only needs IS_HASWELL(dev_priv)
>
> old habits...
Thanks to your old habits I didn't have to make any changes when pushing
this to drm-intel-fixes which is still old habit land. Thanks for the
patch and review.
BR,
Jani.
>
>>
>> > + return true;
>> > +
>> > + return ilk_pipe_pixel_rate(pipe_config) <=
>> > + intel_ddi_get_cdclk_freq(dev_priv) * 95 / 100;
>>
>> Otherwise
>> Acked-by: Chris Wilson
>> -Chris
>>
>> --
>> Chris Wilson, Intel Open Source Technology Centre
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Add rotation support for cursor plane
From: Ville Syrjälä
The cursor plane also supports 180 degree rotation. Add a new
"cursor-rotation" property on the crtc which controls this.
Unlike sprites, the cursor has a fixed size, so if you have a small
cursor image with the rest of the bo filled by transparent pixels,
simply flipping the rotation property will cause the visible part
of the cursor to shift. This is something to keep in mind when
using cursor rotation.
v2: Fix gen4/vlv by offsetting the base address appropriately
v3: Removing cursor-rotation property and using rotation property on cursor
plane.
v4: Changing the author name back to Ville.
Testcase: kms_rotation_crc
Cc: Sagar Kamble
Signed-off-by: Ville Syrjälä
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_display.c | 26 +-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 15c0eaa..5a9fab9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4162,6 +4162,7 @@ enum punit_power_well {
#define MCURSOR_PIPE_A 0x00
#define MCURSOR_PIPE_B (1 << 28)
#define MCURSOR_GAMMA_ENABLE (1 << 26)
+#define CURSOR_ROTATE_180(1<<15)
#define CURSOR_TRICKLE_FEED_DISABLE (1 << 14)
#define _CURABASE 0x70084
#define _CURAPOS 0x70088
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 122ac6e..8c83bcc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8247,6 +8247,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32
base)
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
+ if (to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180))
+ cntl |= CURSOR_ROTATE_180;
+
if (intel_crtc->cursor_cntl != cntl) {
I915_WRITE(CURCNTR(pipe), cntl);
POSTING_READ(CURCNTR(pipe));
@@ -8302,6 +8305,13 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
I915_WRITE(CURPOS(pipe), pos);
+ /* ILK+ do this automagically */
+ if (HAS_GMCH_DISPLAY(dev) &&
+ to_intel_plane(crtc->cursor)->rotation == BIT(DRM_ROTATE_180)) {
+ base += (intel_crtc->cursor_height *
+ intel_crtc->cursor_width - 1) * 4;
+ }
+
if (IS_845G(dev) || IS_I865G(dev))
i845_update_cursor(crtc, base);
else
@@ -8453,7 +8463,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
mutex_unlock(&dev->struct_mutex);
old_width = intel_crtc->cursor_width;
-
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
intel_crtc->cursor_width = width;
@@ -12074,6 +12083,7 @@ static const struct drm_plane_funcs
intel_cursor_plane_funcs = {
.update_plane = intel_cursor_plane_update,
.disable_plane = intel_cursor_plane_disable,
.destroy = intel_plane_destroy,
+ .set_property = intel_plane_set_property,
};
static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -12089,12 +12099,26 @@ static struct drm_plane
*intel_cursor_plane_create(struct drm_device *dev,
cursor->max_downscale = 1;
cursor->pipe = pipe;
cursor->plane = pipe;
+ cursor->rotation = BIT(DRM_ROTATE_0);
drm_universal_plane_init(dev, &cursor->base, 0,
&intel_cursor_plane_funcs,
intel_cursor_formats,
ARRAY_SIZE(intel_cursor_formats),
DRM_PLANE_TYPE_CURSOR);
+
+ if (INTEL_INFO(dev)->gen >= 4) {
+ if (!dev->mode_config.rotation_property)
+ dev->mode_config.rotation_property =
+ drm_mode_create_rotation_property(dev,
+ BIT(DRM_ROTATE_0) |
+ BIT(DRM_ROTATE_180));
+ if (dev->mode_config.rotation_property)
+ drm_object_attach_property(&cursor->base.base,
+ dev->mode_config.rotation_property,
+ cursor->rotation);
+ }
+
return &cursor->base;
}
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Update plane parameters for cursor plane
Please look into this first:
https://bugs.freedesktop.org/show_bug.cgi?id=83130
BR,
Jani.
On Mon, 15 Sep 2014, sonika.jin...@intel.com wrote:
> From: Sonika Jindal
>
> This allows the cursor plane to be updated the same way as primary and
> sprites,
> and same set_property handler is used for all of these planes.
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 842a5e1..122ac6e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12019,6 +12019,22 @@ intel_cursor_plane_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> };
> + const struct {
> + int crtc_x, crtc_y;
> + unsigned int crtc_w, crtc_h;
> + uint32_t src_x, src_y, src_w, src_h;
> + } orig = {
> + .crtc_x = crtc_x,
> + .crtc_y = crtc_y,
> + .crtc_w = crtc_w,
> + .crtc_h = crtc_h,
> + .src_x = src_x,
> + .src_y = src_y,
> + .src_w = src_w,
> + .src_h = src_h,
> + };
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> bool visible;
> int ret;
>
> @@ -12032,6 +12048,17 @@ intel_cursor_plane_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
>
> crtc->cursor_x = crtc_x;
> crtc->cursor_y = crtc_y;
> +
> + intel_plane->crtc_x = orig.crtc_x;
> + intel_plane->crtc_y = orig.crtc_y;
> + intel_plane->crtc_w = orig.crtc_w;
> + intel_plane->crtc_h = orig.crtc_h;
> + intel_plane->src_x = orig.src_x;
> + intel_plane->src_y = orig.src_y;
> + intel_plane->src_w = orig.src_w;
> + intel_plane->src_h = orig.src_h;
> + intel_plane->obj = obj;
> +
> if (fb != crtc->cursor->fb) {
> return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
> } else {
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] uxa: intel_sync_close() is only available when HAVE_DRI3
On Sat, Sep 13, 2014 at 07:45:01PM +0200, Sedat Dilek wrote: > With LLVM v3.4.2 I got this error reported: > ... > intel_driver.c:1182:2: error: implicit declaration of function > 'intel_sync_close' is invalid in C99 [-Werror,-Wimplicit-function-declaration] > intel_sync_close(screen); > ^ > In file included from intel_uxa.c:44: > ./intel_glamor.h:92:1: warning: unused function 'intel_glamor_fd_from_pixmap' > [-Wunused-function] > intel_glamor_fd_from_pixmap(ScreenPtr screen, > ^ > intel_driver.c:1182:2: note: did you mean 'intel_mode_close'? > ./intel.h:356:13: note: 'intel_mode_close' declared here > extern void intel_mode_close(intel_screen_private *intel); > ... > > Looking at intel_sync_close() is only available when DRI3 is > supported. > > 516: #if HAVE_DRI3 > 517: Bool intel_sync_init(ScreenPtr screen); > 518: void intel_sync_close(ScreenPtr screen); > 519: #endif > > Fix the issue by embedding intel_sync_close() with a HAVE_DRI3 ifdef check. > > Signed-off-by: Sedat Dilek I went with a slightly different approach to keep the ifdefery out of the body: commit 067115a51b2646538a38ba603c688233c61e23cd Author: Chris Wilson Date: Mon Sep 15 08:44:41 2014 +0100 uxa: Stub out intel_sync_init|fini when not compiled in In order to fix the build without DRI3, we need to stub out the functions not compiled in, such as intel_sync_fini(). Reported-by: Sedat Dilek Signed-off-by: Chris Wilson Thanks for the bug report and 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 14/19] drm: Don't update vblank timestamp when the counter didn't change
On Sat, Sep 13, 2014 at 06:25:54PM +0200, Mario Kleiner wrote: > The current drm-next misses Ville's original Patch 14/19, the one i first > objected, then objected to my objection. It is needed to avoid actual > regressions. Attached a trivially rebased (v2) of Ville's patch to go on top > of drm-next, also as tgz in case my e-mail client mangles the patch again, > because it's one of those "email hates me" weeks. Oh dear, I've made a decent mess of all of this really. Picked up to make sure it doesn't get lost again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] topic/core-stuff
Hi Dave, Here's the updated topic/core-stuff pull request with the two patches already merged into drm-fixes dropped. Cheers, Daniel The following changes since commit edbaae5a5cab89de0e64b8c03ebd9a8d5d266550: Merge tag 'topic/vblank-rework-2014-09-12' of git://anongit.freedesktop.org/drm-intel into drm-next (2014-09-12 19:04:53 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-09-15 for you to fetch changes up to d0fa1af40e784aaf7ebb7ba8a17b229bb3fa4c21: drm: Drop modeset locking from crtc init function (2014-09-15 08:56:30 +0200) Chris Wilson (1): drm: Include task->name and master status in debugfs clients info Clint Taylor (2): drm/edid: Reduce horizontal timings for pixel replicated modes drm/i915/hdmi: Enable pipe pixel replication for SD interlaced modes Daniel Vetter (1): drm: Drop modeset locking from crtc init function Julia Lawall (1): drm: use c99 initializers in structures Laurent Pinchart (1): drm/gem: Fix kerneldoc typo Randy Dunlap (1): drm: fix drm_modeset_lock.h kernel-doc notation drivers/gpu/drm/drm_crtc.c| 5 -- drivers/gpu/drm/drm_edid.c| 117 +++--- drivers/gpu/drm/drm_gem.c | 2 +- drivers/gpu/drm/drm_info.c| 27 +++-- drivers/gpu/drm/i915/intel_hdmi.c | 15 - drivers/gpu/drm/sti/sti_vtac.c| 12 +++- include/drm/drm_modeset_lock.h| 4 +- 7 files changed, 107 insertions(+), 75 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix Sink CRC
On Fri, Sep 12, 2014 at 07:49:25PM -0400, Rodrigo Vivi wrote:
> In some cases like when PSR just got enabled the panel need more vblank
> times to calculate CRC. I figured that out with the new PSR test cases
> facing some cases that I had a green screen but a blank CRC. Even with
> 2 vblank waits on kernel + 2 vblank waits on test case.
>
> So let's give up to 6 vblank wait time. However we now check for
> TEST_CRC_COUNT that shows when panel finished to calculate CRC and
> has it ready.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_dp.c | 20 ++--
> include/drm/drm_dp_helper.h | 5 +++--
> 2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f79473b..eda6467 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3468,21 +3468,29 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
> *crc)
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct intel_crtc *intel_crtc =
> to_intel_crtc(intel_dig_port->base.base.crtc);
> - u8 buf[1];
> + u8 buf;
> + int test_crc_count;
> + int attempts = 6;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0)
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> return -EAGAIN;
>
> - if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
> + if (!(buf & DP_TEST_CRC_SUPPORTED))
> return -ENOTTY;
>
> + test_crc_count = buf & DP_TEST_COUNT_MASK;
> +
> if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> DP_TEST_SINK_START) < 0)
> return -EAGAIN;
>
> - /* Wait 2 vblanks to be sure we will have the correct CRC value */
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> + do {
> + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + } while(attempts-- && (buf & DP_TEST_COUNT_MASK) <= test_crc_count);
Shouldn't this here fest for (buf & MAS) != test_crc_count?
-Daniel
> +
> + if (attempts == 0)
> + return -EAGAIN;
>
> if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
> return -EAGAIN;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 9305c71..8edeed0 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -303,7 +303,8 @@
> #define DP_TEST_CRC_B_CB 0x244
>
> #define DP_TEST_SINK_MISC0x246
> -#define DP_TEST_CRC_SUPPORTED(1 << 5)
> +# define DP_TEST_CRC_SUPPORTED (1 << 5)
> +# define DP_TEST_COUNT_MASK 0x7
>
> #define DP_TEST_RESPONSE 0x260
> # define DP_TEST_ACK (1 << 0)
> @@ -313,7 +314,7 @@
> #define DP_TEST_EDID_CHECKSUM0x261
>
> #define DP_TEST_SINK 0x270
> -#define DP_TEST_SINK_START (1 << 0)
> +# define DP_TEST_SINK_START (1 << 0)
>
> #define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */
> # define DP_PAYLOAD_TABLE_UPDATED (1 << 0)
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915: Increase PSR Idle Frame to 2.
On Fri, Sep 12, 2014 at 05:29:44PM -0700, Rodrigo Vivi wrote: > Please ignore this full series here. > > I have a better one that let PSR on HSW in a better stage with only 1 idle > frame and without changing the 100ms. Actually if we are brave we can > reduce this to 24 or less. The new work is currently under > http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=psr-3.18 > However I'm not going to send yet because on BDW it got works. It seems BDW > doesn't like to get PSR enabled immediatelly. So I'm justs sendint new > series after I'm confortable that PSR is in a better stage for both HSW and > BDW. How does this interact with the improved psr crc capture code? And if we aim for perfect we should be able to get the delay for the reenable for down to 0. 24ms is still about 1.5 frames ... But sounding much better indeed now, nice work! -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 2/4] drm/i915: Only set CURSOR_PIPE_CSC_ENABLE when cursor is enabled
On Sat, Sep 13, 2014 at 05:17:29PM +0100, Chris Wilson wrote: > On Fri, Sep 12, 2014 at 08:53:33PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > It seems cleaner if we keep CURCNTR at 0 when the cursor is disabled, > > so don't set the CURSOR_PIPE_CSC_ENABLE bit unless the cursor is > > enabled. > > > > Signed-off-by: Ville Syrjälä > Reviewed-by: Chris Wilson Merged the first 2 patches from this series to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH igt 1/2] lib: Add igt_plane_set_size()
On Fri, Sep 12, 2014 at 09:38:13PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Allow tests to specify the plane size instead of assuming that the
> entire FB will be scanned out.
>
> To keep the current tests working without having to sprinkle
> igt_plane_set_size() calls all over the place, make
> igt_plane_set_fb() reset the plane size to the FB size.
Can you please add gtkdoc for just these two functions? We need to start
somewhere ...
Thanks, Daniel
>
> Signed-off-by: Ville Syrjälä
> ---
> lib/igt_kms.c | 34 ++
> lib/igt_kms.h | 3 +++
> 2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 3a850f1..0547147 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1269,7 +1269,7 @@ static int igt_drm_plane_commit(igt_plane_t *plane,
> fb_id,
> 0,/* flags */
> plane->crtc_x, plane->crtc_y,
> - plane->fb->width, plane->fb->height,
> + plane->crtc_w, plane->crtc_h,
> IGT_FIXED(0,0), /* src_x */
> IGT_FIXED(0,0), /* src_y */
> IGT_FIXED(plane->fb->width,0), /* src_w */
> @@ -1314,12 +1314,12 @@ static int igt_cursor_commit_legacy(igt_plane_t
> *cursor,
> igt_output_name(output),
> kmstest_pipe_name(output->config.pipe),
> gem_handle,
> - cursor->fb->width, cursor->fb->height);
> + cursor->crtc_w, cursor->crtc_h);
>
> ret = drmModeSetCursor(display->drm_fd, crtc_id,
> gem_handle,
> -cursor->fb->width,
> -cursor->fb->height);
> +cursor->crtc_w,
> +cursor->crtc_h);
> } else {
> LOG(display,
> "%s: SetCursor pipe %s, disabling\n",
> @@ -1631,6 +1631,14 @@ void igt_plane_set_fb(igt_plane_t *plane, struct
> igt_fb *fb)
> plane->index, fb ? fb->fb_id : 0);
>
> plane->fb = fb;
> + /* hack to keep tests working that don't call igt_plane_set_size() */
> + if (fb) {
> + plane->crtc_w = fb->width;
> + plane->crtc_h = fb->height;
> + } else {
> + plane->crtc_w = 0;
> + plane->crtc_h = 0;
> + }
>
> plane->fb_changed = true;
> }
> @@ -1649,6 +1657,24 @@ void igt_plane_set_position(igt_plane_t *plane, int x,
> int y)
> plane->position_changed = true;
> }
>
> +void igt_plane_set_size(igt_plane_t *plane, int w, int h)
> +{
> + igt_pipe_t *pipe = plane->pipe;
> + igt_display_t *display = pipe->display;
> +
> + LOG(display, "%s.%d: plane_set_size(%d,%d)\n",
> + kmstest_pipe_name(pipe->pipe), plane->index, w, h);
> +
> + plane->crtc_w = w;
> + plane->crtc_h = h;
> +
> + /*
> + * must be fb_changed so that legacy cursors call
> + * drmModeSetCursor() instead of drmModeMoveCursor()
> + */
> + plane->fb_changed = true;
> +}
> +
> void igt_plane_set_panning(igt_plane_t *plane, int x, int y)
> {
> igt_pipe_t *pipe = plane->pipe;
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 921afef..027b4e0 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -212,6 +212,8 @@ typedef struct {
>
> /* position within pipe_src_w x pipe_src_h */
> int crtc_x, crtc_y;
> + /* size within pipe_src_w x pipe_src_h */
> + int crtc_w, crtc_h;
> /* panning offset within the fb */
> unsigned int pan_x, pan_y;
> igt_rotation_t rotation;
> @@ -266,6 +268,7 @@ static inline bool
> igt_plane_supports_rotation(igt_plane_t *plane)
>
> void igt_plane_set_fb(igt_plane_t *plane, struct igt_fb *fb);
> void igt_plane_set_position(igt_plane_t *plane, int x, int y);
> +void igt_plane_set_size(igt_plane_t *plane, int w, int h);
> void igt_plane_set_panning(igt_plane_t *plane, int x, int y);
> void igt_plane_set_rotation(igt_plane_t *plane, igt_rotation_t rotation);
>
> --
> 1.8.5.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add limited color range readout for HDMI/DP ports on g4x/vlv/chv
On Fri, 12 Sep 2014, Chris Clayton wrote:
> On 09/12/14 13:46, ville.syrj...@linux.intel.com wrote:
>> From: Ville Syrjälä
>>
>> The limited color range knob is in the port registers on
>> g4x and vlv/chv for HDMI, and on g4x for DP. Add the relevant code
>> to read out the hardware state into pipe config. On vlv/chv the
>> DP port limited color range knob is in PIPECONF for which we
>> already have readout code.
>>
> With the patch below applied, the warning has gone away. My thanks to Daniel
> and Ville.
>
> Tested-by: Chris Clayton
Pushed to drm-intel-fixes with Daniel's IRC reviewed-by. Thanks for the
patch and testing.
BR,
Jani.
>
>> Cc: Chris Clayton
>> Signed-off-by: Ville Syrjälä
>> ---
>> drivers/gpu/drm/i915/intel_dp.c | 4
>> drivers/gpu/drm/i915/intel_hdmi.c | 7 ++-
>> 2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index d8e868a..d73b4b7 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1910,6 +1910,10 @@ static void intel_dp_get_config(struct intel_encoder
>> *encoder,
>>
>> pipe_config->adjusted_mode.flags |= flags;
>>
>> +if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev) &&
>> +tmp & DP_COLOR_RANGE_16_235)
>> +pipe_config->limited_color_range = true;
>> +
>> pipe_config->has_dp_encoder = true;
>>
>> intel_dp_get_m_n(crtc, pipe_config);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index c586173..8199371 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -712,7 +712,8 @@ static void intel_hdmi_get_config(struct intel_encoder
>> *encoder,
>>struct intel_crtc_config *pipe_config)
>> {
>> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>> -struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>> +struct drm_device *dev = encoder->base.dev;
>> +struct drm_i915_private *dev_priv = dev->dev_private;
>> u32 tmp, flags = 0;
>> int dotclock;
>>
>> @@ -734,6 +735,10 @@ static void intel_hdmi_get_config(struct intel_encoder
>> *encoder,
>> if (tmp & HDMI_MODE_SELECT_HDMI)
>> pipe_config->has_audio = true;
>>
>> +if (!HAS_PCH_SPLIT(dev) &&
>> +tmp & HDMI_COLOR_RANGE_16_235)
>> +pipe_config->limited_color_range = true;
>> +
>> pipe_config->adjusted_mode.flags |= flags;
>>
>> if ((tmp & SDVO_COLOR_FORMAT_MASK) == HDMI_COLOR_FORMAT_12bpc)
>>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915/ppgtt: Load address space after mi_set_context
From: Ben Widawsky
The simple explanation is, the docs say to do this for GEN8. Perhaps we
want to do this for GEN7 too, I am not certain.
PDPs are saved and restored with context. Contexts (without execlists)
only exist on the render ring. The docs say that PDPs are not power
context save/restored. I've learned that this actually means something
which SW doesn't care about. So pretend the statement doesn't exist.
For non RCS, nothing changes.
All this patch now does is change the ordering of LRI vs MI_SET_CONTEXT
for the initialization of the context. I do this because the docs say to
do it, and frankly, I cannot reason why it is necessary. I've thought
about it a lot, and tried, without success, to get a reason from design.
The answer I got more or less says, "gen7 is different than gen8." I've
given up, and am adding this little bit of code to make it in sync with
the docs.
v2: Completely rewritten commit message that addresses the requests
Ville made for v1
Only load PDPs for initial context load (Ville)
v3: Rebased after ppgtt clean-up rules, and apply only for render
ring. This is needed to boot to desktop with full ppgtt in legacy mode
(without execlists).
v4: Clean up the pre/post load logic (Ville).
Signed-off-by: Ben Widawsky
Signed-off-by: Michel Thierry (v3-4)
---
drivers/gpu/drm/i915/i915_gem_context.c | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index a5221d8..7b73b36 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -522,6 +522,9 @@ static int do_switch(struct intel_engine_cs *ring,
struct intel_context *from = ring->last_context;
u32 hw_flags = 0;
bool uninitialized = false;
+ bool needs_pd_load_pre = ((INTEL_INFO(ring->dev)->gen < 8) ||
+ (ring != &dev_priv->ring[RCS])) && to->ppgtt;
+ bool needs_pd_load_post = false;
int ret, i;
if (from != NULL && ring == &dev_priv->ring[RCS]) {
@@ -547,7 +550,11 @@ static int do_switch(struct intel_engine_cs *ring,
*/
from = ring->last_context;
- if (to->ppgtt) {
+ if (needs_pd_load_pre) {
+ /* Older GENs and non render rings still want the load first,
+* "PP_DCLV followed by PP_DIR_BASE register through Load
+* Register Immediate commands in Ring Buffer before submitting
+* a context."*/
ret = to->ppgtt->switch_mm(to->ppgtt, ring);
if (ret)
goto unpin_out;
@@ -577,13 +584,34 @@ static int do_switch(struct intel_engine_cs *ring,
vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level,
GLOBAL_BIND);
}
- if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+ /* GEN8 does *not* require an explicit reload if the PDPs have been
+* setup, and we do not wish to move them.
+*
+* XXX: If we implemented page directory eviction code, this
+* optimization needs to be removed.
+*/
+ if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) {
hw_flags |= MI_RESTORE_INHIBIT;
+ needs_pd_load_post = to->ppgtt && IS_GEN8(ring->dev);
+ }
ret = mi_set_context(ring, to, hw_flags);
if (ret)
goto unpin_out;
+ if (needs_pd_load_post) {
+ ret = to->ppgtt->switch_mm(to->ppgtt, ring);
+ /* The hardware context switch is emitted, but we haven't
+* actually changed the state - so it's probably safe to bail
+* here. Still, let the user know something dangerous has
+* happened.
+*/
+ if (ret) {
+ DRM_ERROR("Failed to change address space on context
switch\n");
+ goto unpin_out;
+ }
+ }
+
for (i = 0; i < MAX_L3_SLICES; i++) {
if (!(to->remap_slice & (1
[Intel-gfx] [PATCH 2/2] drm/i915: Fix irq checks in ring->irq_get/put functions
Yet another place that wasn't properly transformed when implementing
SOix. While at it convert the checks to WARN_ON on gen5+ (since we
don't have UMS potentially doing stupid things on those platforms).
And also add the corresponding checks to the put functions (again with
a WARN_ON) for gen5+.
Cc: Imre Deak
Cc: Jesse Barnes
Cc: "Volkin, Bradley D"
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_lrc.c| 4 +++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 23 ++-
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index bafd38b5703e..5fb61d76fd60 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1063,7 +1063,7 @@ static bool gen8_logical_ring_get_irq(struct
intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1082,6 +1082,8 @@ static void gen8_logical_ring_put_irq(struct
intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
+ WARN_ON(!intel_irqs_enabled(dev_priv));
+
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
I915_WRITE_IMR(ring, ~ring->irq_keep_mask);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 264d2827b2fb..b568c1f2c57b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1197,7 +1197,7 @@ gen5_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1215,6 +1215,8 @@ gen5_ring_put_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
+ WARN_ON(!intel_irqs_enabled(dev_priv));
+
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0)
gen5_disable_gt_irq(dev_priv, ring->irq_enable_mask);
@@ -1228,7 +1230,7 @@ i9xx_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (!intel_irqs_enabled(dev_priv))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1265,7 +1267,7 @@ i8xx_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (!intel_irqs_enabled(dev_priv))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1399,8 +1401,8 @@ gen6_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
- return false;
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+ return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
@@ -1424,6 +1426,8 @@ gen6_ring_put_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
+ WARN_ON(!intel_irqs_enabled(dev_priv));
+
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
if (HAS_L3_DPF(dev) && ring->id == RCS)
@@ -1442,7 +1446,7 @@ hsw_vebox_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1462,8 +1466,7 @@ hsw_vebox_put_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
- return;
+ WARN_ON(!intel_irqs_enabled(dev_priv));
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
@@ -1480,7 +1483,7 @@ gen8_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1506,6 +1509,8 @@ gen8_ring_put_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_p
[Intel-gfx] [PATCH 1/2] drm/i915: Fix irq_enabled checks in vlv display irq enable/disable
In our suspend/resume and driver load code we enable power wells and interrupts in different order than with runtime pm, which means code needs to check for that and act accordingly. Unfortunately with the SOiX support the "are interrupts enabled" checks went out of sync. Fix up more places. This specific one was caught by the recently added interrupt checks in pipestate_enable/disable functions. This resulted in the following backtrace Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. Suspending console(s) (use no_console_suspend to debug) sd 0:0:0:0: [sda] Synchronizing SCSI cache sd 0:0:0:0: [sda] Stopping disk [ cut here ] WARNING: CPU: 1 PID: 20572 at drivers/gpu/drm/i915/i915_irq.c:619 i915_disable_pipestat+0x94/0x122 [i915]() Modules linked in: dm_mod iTCO_wdt iTCO_vendor_support snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep pcspkr snd_pcm serio_raw snd_timer i2c_i801 lpc_ich snd mfd_core soundcore iosf_mbi battery ac acpi_cpufreq i915 button video drm_kms_helper drm CPU: 1 PID: 20572 Comm: kworker/u8:2 Tainted: G W 3.17.0-rc4_prts_a44085_20140912_debug+ #62 Workqueue: events_unbound async_run_entry_fn 88006f0dbb18 81815c9c 88006f0dbb50 8103e87f a00b32e1 88000369 1c00 1c00 001f0024 88006f0dbb60 Call Trace: [] dump_stack+0x45/0x56 [] warn_slowpath_common+0x7f/0x98 [] ? i915_disable_pipestat+0x94/0x122 [i915] [] warn_slowpath_null+0x1a/0x1c [] i915_disable_pipestat+0x94/0x122 [i915] [] valleyview_display_irqs_uninstall+0x99/0x101 [i915] [] valleyview_disable_display_irqs+0x37/0x39 [i915] [] vlv_display_power_well_disable+0x53/0x79 [i915] [] intel_display_power_put+0xe3/0x111 [i915] [] intel_display_set_init_power+0x31/0x3d [i915] [] i915_drm_freeze+0x1c0/0x1cd [i915] [] i915_pm_suspend+0x44/0x46 [i915] [] pci_pm_suspend+0x85/0x106 [] ? pci_pm_freeze+0xb1/0xb1 [] dpm_run_callback+0x43/0xd3 [] __device_suspend+0x1e3/0x263 [] async_suspend+0x1f/0x8a [] async_run_entry_fn+0x61/0x10b [] process_one_work+0x221/0x3f2 [] ? process_one_work+0x1ac/0x3f2 [] worker_thread+0x288/0x37c [] ? cancel_delayed_work_sync+0x15/0x15 [] kthread+0xf6/0xfe [] ? kthread_create_on_node+0x19a/0x19a [] ret_from_fork+0x7c/0xb0 [] ? kthread_create_on_node+0x19a/0x19a ---[ end trace f0b4cc6544a9afea ]--- Cc: Imre Deak Cc: Jesse Barnes Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 7db0029558a4..20a5d6150919 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3723,7 +3723,7 @@ void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv) dev_priv->display_irqs_enabled = true; - if (dev_priv->dev->irq_enabled) + if (dev_priv->pm._irqs_disabled) valleyview_display_irqs_install(dev_priv); } @@ -3736,7 +3736,7 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv) dev_priv->display_irqs_enabled = false; - if (dev_priv->dev->irq_enabled) + if (dev_priv->pm._irqs_disabled) valleyview_display_irqs_uninstall(dev_priv); } -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4] drm/i915/ppgtt: Load address space after mi_set_context
On Mon, Sep 15, 2014 at 10:29:47AM +0100, Michel Thierry wrote:
> From: Ben Widawsky
>
> The simple explanation is, the docs say to do this for GEN8. Perhaps we
> want to do this for GEN7 too, I am not certain.
>
> PDPs are saved and restored with context. Contexts (without execlists)
> only exist on the render ring. The docs say that PDPs are not power
> context save/restored. I've learned that this actually means something
> which SW doesn't care about. So pretend the statement doesn't exist.
> For non RCS, nothing changes.
>
> All this patch now does is change the ordering of LRI vs MI_SET_CONTEXT
> for the initialization of the context. I do this because the docs say to
> do it, and frankly, I cannot reason why it is necessary. I've thought
> about it a lot, and tried, without success, to get a reason from design.
> The answer I got more or less says, "gen7 is different than gen8." I've
> given up, and am adding this little bit of code to make it in sync with
> the docs.
>
> v2: Completely rewritten commit message that addresses the requests
> Ville made for v1
> Only load PDPs for initial context load (Ville)
>
> v3: Rebased after ppgtt clean-up rules, and apply only for render
> ring. This is needed to boot to desktop with full ppgtt in legacy mode
> (without execlists).
>
> v4: Clean up the pre/post load logic (Ville).
>
> Signed-off-by: Ben Widawsky
> Signed-off-by: Michel Thierry (v3-4)
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 32 ++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index a5221d8..7b73b36 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -522,6 +522,9 @@ static int do_switch(struct intel_engine_cs *ring,
> struct intel_context *from = ring->last_context;
> u32 hw_flags = 0;
> bool uninitialized = false;
> + bool needs_pd_load_pre = ((INTEL_INFO(ring->dev)->gen < 8) ||
> + (ring != &dev_priv->ring[RCS])) && to->ppgtt;
> + bool needs_pd_load_post = false;
> int ret, i;
>
> if (from != NULL && ring == &dev_priv->ring[RCS]) {
> @@ -547,7 +550,11 @@ static int do_switch(struct intel_engine_cs *ring,
>*/
> from = ring->last_context;
>
> - if (to->ppgtt) {
> + if (needs_pd_load_pre) {
> + /* Older GENs and non render rings still want the load first,
> + * "PP_DCLV followed by PP_DIR_BASE register through Load
> + * Register Immediate commands in Ring Buffer before submitting
> + * a context."*/
What's the reference for this? It works if you load the ppgtt after the
set_context on ivb/byt/hsw, so do you have a specific recommendation in
mind? And the set-context even provides the barrier required for the lri.
> ret = to->ppgtt->switch_mm(to->ppgtt, ring);
> if (ret)
> goto unpin_out;
> @@ -577,13 +584,34 @@ static int do_switch(struct intel_engine_cs *ring,
> vma->bind_vma(vma, to->legacy_hw_ctx.rcs_state->cache_level,
> GLOBAL_BIND);
> }
>
> - if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
> + /* GEN8 does *not* require an explicit reload if the PDPs have been
> + * setup, and we do not wish to move them.
> + *
> + * XXX: If we implemented page directory eviction code, this
> + * optimization needs to be removed.
> + */
What is the cost of the pde invalidate on top of the context restore
anyway? As this will be a secondary path in future, is optimizing
worthwhile at all?
> + if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to)) {
> hw_flags |= MI_RESTORE_INHIBIT;
> + needs_pd_load_post = to->ppgtt && IS_GEN8(ring->dev);
> + }
>
> ret = mi_set_context(ring, to, hw_flags);
> if (ret)
> goto unpin_out;
>
> + if (needs_pd_load_post) {
> + ret = to->ppgtt->switch_mm(to->ppgtt, ring);
> + /* The hardware context switch is emitted, but we haven't
> + * actually changed the state - so it's probably safe to bail
> + * here. Still, let the user know something dangerous has
> + * happened.
> + */
Imagine if we only had patches posted already to fix all of this up.
-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: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 11:41:19AM +0200, Daniel Vetter wrote: > Yet another place that wasn't properly transformed when implementing > SOix. While at it convert the checks to WARN_ON on gen5+ (since we > don't have UMS potentially doing stupid things on those platforms). > And also add the corresponding checks to the put functions (again with > a WARN_ON) for gen5+. > > Cc: Imre Deak > Cc: Jesse Barnes > Cc: "Volkin, Bradley D" > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_lrc.c| 4 +++- > drivers/gpu/drm/i915/intel_ringbuffer.c | 23 ++- > 2 files changed, 17 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index bafd38b5703e..5fb61d76fd60 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1063,7 +1063,7 @@ static bool gen8_logical_ring_get_irq(struct > intel_engine_cs *ring) > struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long flags; > > - if (!dev->irq_enabled) > + if (WARN_ON(!intel_irqs_enabled(dev_priv))) > return false; > > spin_lock_irqsave(&dev_priv->irq_lock, flags); > @@ -1082,6 +1082,8 @@ static void gen8_logical_ring_put_irq(struct > intel_engine_cs *ring) > struct drm_i915_private *dev_priv = dev->dev_private; > unsigned long flags; > > + WARN_ON(!intel_irqs_enabled(dev_priv)); Please no. This would be a BUG(). -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: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 11:48 AM, Chris Wilson wrote: > intel_engine_cs *ring) >> struct drm_i915_private *dev_priv = dev->dev_private; >> unsigned long flags; >> >> + WARN_ON(!intel_irqs_enabled(dev_priv)); > > Please no. This would be a BUG(). No BUG if not doing it means the driver will survive for a bit longer. And doing a few bogus register writes usually means it'll surive. Similar checks I've added just recently to pipestate_enable/disable caught a bug in the resume code. Using a BUG instead of WARN would have meant some serious debugging, but as-is a look at the backtrace was all that was needed to analyze the bug. I know a lot of developers disagree, but debugging random crap in the field is _so_ much easier with WARN than BUG that it's not even up for discussion imo. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: vlv: fix display IRQ enable/disable
On Mon, Sep 08, 2014 at 03:21:09PM +0300, Imre Deak wrote: > We want to enable/disable display IRQs only if global i915 IRQs are > enabled. To check the latter it's not enough to consult the DRM > dev->irq_enabled flag, since runtime PM can disable/enable IRQs > and it won't adjust this flag only the i915 specific > dev_priv->pm._irqs_disabled flag. Fix this by using the proper > intel_irqs_enabled() helper instead. > > Fortunately this didn't cause an actual problem since even if we enabled > display IRQs too early (before enabling global i915 IRQs) the > VLV_MASTER_IER would still be clear masking all IRQs. > > This issue was caught by > > commit 920dd15a2b2fc60d054646a8a1ffd6aeb6090e05 > Author: Daniel Vetter > Date: Wed Aug 27 10:43:37 2014 +0200 > > drm/i915: WARN if interrupts aren't on in en/disable_pipestat > > Signed-off-by: Imre Deak Please cc relevant people next time around, for this Jesse (since this all goes down to his soix enabling). Thanks for the patch, yours here is better than the one I've just posted in response to prts. Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index d6cf1b7..14f52ac 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3723,7 +3723,7 @@ void valleyview_enable_display_irqs(struct > drm_i915_private *dev_priv) > > dev_priv->display_irqs_enabled = true; > > - if (dev_priv->dev->irq_enabled) > + if (intel_irqs_enabled(dev_priv)) > valleyview_display_irqs_install(dev_priv); > } > > @@ -3736,7 +3736,7 @@ void valleyview_disable_display_irqs(struct > drm_i915_private *dev_priv) > > dev_priv->display_irqs_enabled = false; > > - if (dev_priv->dev->irq_enabled) > + if (intel_irqs_enabled(dev_priv)) > valleyview_display_irqs_uninstall(dev_priv); > } > > -- > 1.8.4 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 11:51:42AM +0200, Daniel Vetter wrote: > On Mon, Sep 15, 2014 at 11:48 AM, Chris Wilson > wrote: > > intel_engine_cs *ring) > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> unsigned long flags; > >> > >> + WARN_ON(!intel_irqs_enabled(dev_priv)); > > > > Please no. This would be a BUG(). > > No BUG if not doing it means the driver will survive for a bit longer. > And doing a few bogus register writes usually means it'll surive. I mean that this offers no additional benefit over the first WARN. Which is already redundant as we WARN in the caller. There are more meaningful places where that WARN would be appropriate, but after the event of something else screwing up is usually close to useless. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix Sink CRC
On Sat, 13 Sep 2014, Rodrigo Vivi wrote:
> In some cases like when PSR just got enabled the panel need more vblank
> times to calculate CRC. I figured that out with the new PSR test cases
> facing some cases that I had a green screen but a blank CRC. Even with
> 2 vblank waits on kernel + 2 vblank waits on test case.
>
> So let's give up to 6 vblank wait time. However we now check for
> TEST_CRC_COUNT that shows when panel finished to calculate CRC and
> has it ready.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_dp.c | 20 ++--
> include/drm/drm_dp_helper.h | 5 +++--
> 2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f79473b..eda6467 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3468,21 +3468,29 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
> *crc)
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct intel_crtc *intel_crtc =
> to_intel_crtc(intel_dig_port->base.base.crtc);
> - u8 buf[1];
> + u8 buf;
> + int test_crc_count;
> + int attempts = 6;
>
> - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0)
> + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> return -EAGAIN;
>
> - if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
> + if (!(buf & DP_TEST_CRC_SUPPORTED))
> return -ENOTTY;
>
> + test_crc_count = buf & DP_TEST_COUNT_MASK;
> +
> if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
> DP_TEST_SINK_START) < 0)
> return -EAGAIN;
>
> - /* Wait 2 vblanks to be sure we will have the correct CRC value */
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> - intel_wait_for_vblank(dev, intel_crtc->pipe);
> + do {
> + drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + } while(attempts-- && (buf & DP_TEST_COUNT_MASK) <= test_crc_count);
> +
> + if (attempts == 0)
> + return -EAGAIN;
If the do-while stops because of attempts, we'll never end up here
because of the post-decrement. (We'll only return -EAGAIN here if the
other condition does not hold at precisely attempts == 1 before the
post-decrement.)
BR,
Jani.
>
> if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
> return -EAGAIN;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 9305c71..8edeed0 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -303,7 +303,8 @@
> #define DP_TEST_CRC_B_CB 0x244
>
> #define DP_TEST_SINK_MISC0x246
> -#define DP_TEST_CRC_SUPPORTED(1 << 5)
> +# define DP_TEST_CRC_SUPPORTED (1 << 5)
> +# define DP_TEST_COUNT_MASK 0x7
>
> #define DP_TEST_RESPONSE 0x260
> # define DP_TEST_ACK (1 << 0)
> @@ -313,7 +314,7 @@
> #define DP_TEST_EDID_CHECKSUM0x261
>
> #define DP_TEST_SINK 0x270
> -#define DP_TEST_SINK_START (1 << 0)
> +# define DP_TEST_SINK_START (1 << 0)
>
> #define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */
> # define DP_PAYLOAD_TABLE_UPDATED (1 << 0)
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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 2/2] drm/i915: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 11:56 AM, Chris Wilson wrote: > On Mon, Sep 15, 2014 at 11:51:42AM +0200, Daniel Vetter wrote: >> On Mon, Sep 15, 2014 at 11:48 AM, Chris Wilson >> wrote: >> > intel_engine_cs *ring) >> >> struct drm_i915_private *dev_priv = dev->dev_private; >> >> unsigned long flags; >> >> >> >> + WARN_ON(!intel_irqs_enabled(dev_priv)); >> > >> > Please no. This would be a BUG(). >> >> No BUG if not doing it means the driver will survive for a bit longer. >> And doing a few bogus register writes usually means it'll surive. > > I mean that this offers no additional benefit over the first WARN. Which > is already redundant as we WARN in the caller. There are more meaningful > places where that WARN would be appropriate, but after the event of > something else screwing up is usually close to useless. If we drop the runtime pm reference before the put_irq then we'll WARN here. Which is somewhat likely if some waiter doesn't have it's own runtime pm reference but relies upon someone else holding that for him. Then the get_irq will likely happen while the gpu is still busy, but the put_irq has a good chance to only happen once we've cleaned up. Of course it might mean that we get 2 backtraces in some case if e.g. a wait happens somehow before anything is really set up (in driver load). But I think the additional coverage makes this worth it. If it's too annoying we can always back it out again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 12:08:39PM +0200, Daniel Vetter wrote: > On Mon, Sep 15, 2014 at 11:56 AM, Chris Wilson > wrote: > > On Mon, Sep 15, 2014 at 11:51:42AM +0200, Daniel Vetter wrote: > >> On Mon, Sep 15, 2014 at 11:48 AM, Chris Wilson > >> wrote: > >> > intel_engine_cs *ring) > >> >> struct drm_i915_private *dev_priv = dev->dev_private; > >> >> unsigned long flags; > >> >> > >> >> + WARN_ON(!intel_irqs_enabled(dev_priv)); > >> > > >> > Please no. This would be a BUG(). > >> > >> No BUG if not doing it means the driver will survive for a bit longer. > >> And doing a few bogus register writes usually means it'll surive. > > > > I mean that this offers no additional benefit over the first WARN. Which > > is already redundant as we WARN in the caller. There are more meaningful > > places where that WARN would be appropriate, but after the event of > > something else screwing up is usually close to useless. > > If we drop the runtime pm reference before the put_irq then we'll WARN > here. Which is somewhat likely if some waiter doesn't have it's own > runtime pm reference but relies upon someone else holding that for > him. Then the get_irq will likely happen while the gpu is still busy, > but the put_irq has a good chance to only happen once we've cleaned > up. Put the warn before you sleep awaiting the irq. Every other condition is besides the point. You can't detect some other thread breaking in whilst you are asleep and when you are awake the request is either complete or it is not. Reading any registers with the rpm is itself going to generate the WARN so adding yet another WARN afterwards is simply confusing. -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 1/2] drm/i915: Update plane parameters for cursor plane
Ok, let me check this.
Regards,
Sonika
-Original Message-
From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
Sent: Monday, September 15, 2014 1:26 PM
To: Jindal, Sonika; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Update plane parameters for
cursor plane
Please look into this first:
https://bugs.freedesktop.org/show_bug.cgi?id=83130
BR,
Jani.
On Mon, 15 Sep 2014, sonika.jin...@intel.com wrote:
> From: Sonika Jindal
>
> This allows the cursor plane to be updated the same way as primary and
> sprites, and same set_property handler is used for all of these planes.
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_display.c | 27 +++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 842a5e1..122ac6e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12019,6 +12019,22 @@ intel_cursor_plane_update(struct drm_plane *plane,
> struct drm_crtc *crtc,
> .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> };
> + const struct {
> + int crtc_x, crtc_y;
> + unsigned int crtc_w, crtc_h;
> + uint32_t src_x, src_y, src_w, src_h;
> + } orig = {
> + .crtc_x = crtc_x,
> + .crtc_y = crtc_y,
> + .crtc_w = crtc_w,
> + .crtc_h = crtc_h,
> + .src_x = src_x,
> + .src_y = src_y,
> + .src_w = src_w,
> + .src_h = src_h,
> + };
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> +
> bool visible;
> int ret;
>
> @@ -12032,6 +12048,17 @@ intel_cursor_plane_update(struct drm_plane
> *plane, struct drm_crtc *crtc,
>
> crtc->cursor_x = crtc_x;
> crtc->cursor_y = crtc_y;
> +
> + intel_plane->crtc_x = orig.crtc_x;
> + intel_plane->crtc_y = orig.crtc_y;
> + intel_plane->crtc_w = orig.crtc_w;
> + intel_plane->crtc_h = orig.crtc_h;
> + intel_plane->src_x = orig.src_x;
> + intel_plane->src_y = orig.src_y;
> + intel_plane->src_w = orig.src_w;
> + intel_plane->src_h = orig.src_h;
> + intel_plane->obj = obj;
> +
> if (fb != crtc->cursor->fb) {
> return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
> } else {
> --
> 1.7.10.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] uxa: intel_sync_close() is only available when HAVE_DRI3
On Mon, Sep 15, 2014 at 9:58 AM, Chris Wilson wrote: > On Sat, Sep 13, 2014 at 07:45:01PM +0200, Sedat Dilek wrote: >> With LLVM v3.4.2 I got this error reported: >> ... >> intel_driver.c:1182:2: error: implicit declaration of function >> 'intel_sync_close' is invalid in C99 >> [-Werror,-Wimplicit-function-declaration] >> intel_sync_close(screen); >> ^ >> In file included from intel_uxa.c:44: >> ./intel_glamor.h:92:1: warning: unused function >> 'intel_glamor_fd_from_pixmap' [-Wunused-function] >> intel_glamor_fd_from_pixmap(ScreenPtr screen, >> ^ >> intel_driver.c:1182:2: note: did you mean 'intel_mode_close'? >> ./intel.h:356:13: note: 'intel_mode_close' declared here >> extern void intel_mode_close(intel_screen_private *intel); >> ... >> >> Looking at intel_sync_close() is only available when DRI3 is >> supported. >> >> 516: #if HAVE_DRI3 >> 517: Bool intel_sync_init(ScreenPtr screen); >> 518: void intel_sync_close(ScreenPtr screen); >> 519: #endif >> >> Fix the issue by embedding intel_sync_close() with a HAVE_DRI3 ifdef check. >> >> Signed-off-by: Sedat Dilek > > I went with a slightly different approach to keep the ifdefery out of > the body: > > commit 067115a51b2646538a38ba603c688233c61e23cd > Author: Chris Wilson > Date: Mon Sep 15 08:44:41 2014 +0100 > > uxa: Stub out intel_sync_init|fini when not compiled in > > In order to fix the build without DRI3, we need to stub out the > functions not compiled in, such as intel_sync_fini(). > > Reported-by: Sedat Dilek > Signed-off-by: Chris Wilson > > Thanks for the bug report and patch, Great! I was thinking of adding stubs later, but I needed a fast dirty hack. Thanks for the quick fix! - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm: Improve debug output for drm_wait_one_vblank
This replicates what we've done in i915 in commit 31e4b89acbd7b19c9a8557e6e660a583a0b97daa Author: Damien Lespiau Date: Mon Aug 18 13:51:00 2014 +0100 drm/i915: Print the pipe on which the vblank wait times out to make sure that when we switch i915 to drm_wait_one_vblank that the debug output doesn't regress. Cc: Damien Lespiau Cc: Thomas Wood Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index e73cbdaa18df..5ef03c216a27 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1077,7 +1077,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int crtc) u32 last; ret = drm_vblank_get(dev, crtc); - if (WARN_ON(ret)) + if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", crtc, ret)) return; last = drm_vblank_count(dev, crtc); @@ -1086,7 +1086,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int crtc) last != drm_vblank_count(dev, crtc), msecs_to_jiffies(100)); - WARN_ON(ret == 0); + WARN(ret == 0, "vblank wait timed out on crtc %i\n", crtc); drm_vblank_put(dev, crtc); } -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Extend GET_APERTURE ioctl to report available map space
When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.
This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.
This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.
The patch also adds a debugfs file for convenient testing and reporting.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 28 +
drivers/gpu/drm/i915/i915_gem.c | 111 ++--
include/uapi/drm/i915_drm.h | 20 +++
3 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e63ccbea52e..41d92f29aef1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -534,6 +534,33 @@ static int obj_rank_by_ggtt(void *priv, struct list_head
*A, struct list_head *B
return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
}
+static int i915_gem_aperture_info(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct drm_i915_gem_get_aperture arg;
+ int ret;
+
+ ret = i915_gem_get_aperture_ioctl(dev, &arg, NULL);
+ if (ret)
+ return ret;
+
+ seq_printf(m, "Total size of the GTT: %llu bytes\n",
+ arg.aper_size);
+ seq_printf(m, "Available space in the GTT: %llu bytes\n",
+ arg.aper_available_size);
+ seq_printf(m, "Available space in the mappable aperture: %u bytes\n",
+ arg.map_available_size);
+ seq_printf(m, "Single largest space in the mappable aperture: %u
bytes\n",
+ arg.map_largest_size);
+ seq_printf(m, "Available space for fences: %u bytes\n",
+ arg.fence_available_size);
+ seq_printf(m, "Single largest fence available: %u bytes\n",
+ arg.fence_largest_size);
+
+ return 0;
+}
+
static int i915_gem_gtt_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
@@ -4198,6 +4225,7 @@ static int i915_debugfs_create(struct dentry *root,
static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
+ {"i915_gem_aperture", i915_gem_aperture_info, 0},
{"i915_gem_gtt", i915_gem_gtt_info, 0},
{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4b9de297b967..4b75086a1dc9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -31,6 +31,7 @@
#include "i915_drv.h"
#include "i915_trace.h"
#include "intel_drv.h"
+#include
#include
#include
#include
@@ -260,6 +261,49 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
return 0;
}
+static int obj_rank_by_ggtt(void *priv,
+ struct list_head *A,
+ struct list_head *B)
+{
+ struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
+ struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
+
+ return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
+}
+
+static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
+{
+ u32 size = end - start;
+ u32 fence_size;
+
+ if (INTEL_INFO(dev_priv)->gen < 4) {
+ u32 fence_max;
+ u32 fence_next;
+
+ if (IS_GEN3(dev_priv)) {
+ fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
+ fence_next = 1024*1024;
+ } else {
+ fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
+ fence_next = 512*1024;
+ }
+
+ fence_max = min(fence_max, size);
+ fence_size = 0;
+ while (fence_next <= fence_max) {
+ u32 base = ALIGN(start,
[Intel-gfx] [PATCH] drm/i915: Fix irq checks in ring->irq_get/put functions
Yet another place that wasn't properly transformed when implementing
SOix. While at it convert the checks to WARN_ON on gen5+ (since we
don't have UMS potentially doing stupid things on those platforms).
And also add the corresponding checks to the put functions (again with
a WARN_ON) for gen5+.
v2: Drop the WARNINGS in the irq_put functions (including the existing
one for vebox), Chris convinced me that they're not that terribly
useful.
v3: Don't forget about execlist code.
Cc: Imre Deak
Cc: Jesse Barnes
Cc: "Volkin, Bradley D"
Cc: Chris Wilson
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_lrc.c| 2 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +++--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index bafd38b5703e..803fc38664c4 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1063,7 +1063,7 @@ static bool gen8_logical_ring_get_irq(struct
intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 264d2827b2fb..799f51241879 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1197,7 +1197,7 @@ gen5_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1228,7 +1228,7 @@ i9xx_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (!intel_irqs_enabled(dev_priv))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1265,7 +1265,7 @@ i8xx_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (!intel_irqs_enabled(dev_priv))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1399,8 +1399,8 @@ gen6_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
- return false;
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
+ return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (ring->irq_refcount++ == 0) {
@@ -1442,7 +1442,7 @@ hsw_vebox_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
@@ -1462,9 +1462,6 @@ hsw_vebox_put_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
- return;
-
spin_lock_irqsave(&dev_priv->irq_lock, flags);
if (--ring->irq_refcount == 0) {
I915_WRITE_IMR(ring, ~0);
@@ -1480,7 +1477,7 @@ gen8_ring_get_irq(struct intel_engine_cs *ring)
struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
- if (!dev->irq_enabled)
+ if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return false;
spin_lock_irqsave(&dev_priv->irq_lock, flags);
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Improve debug output for drm_wait_one_vblank
On Mon, Sep 15, 2014 at 02:05:56PM +0200, Daniel Vetter wrote: > This replicates what we've done in i915 in > > commit 31e4b89acbd7b19c9a8557e6e660a583a0b97daa > Author: Damien Lespiau > Date: Mon Aug 18 13:51:00 2014 +0100 > > drm/i915: Print the pipe on which the vblank wait times out > > to make sure that when we switch i915 to drm_wait_one_vblank that the > debug output doesn't regress. > > Cc: Damien Lespiau > Cc: Thomas Wood > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/drm_irq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index e73cbdaa18df..5ef03c216a27 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -1077,7 +1077,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int > crtc) > u32 last; > > ret = drm_vblank_get(dev, crtc); > - if (WARN_ON(ret)) > + if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", crtc, ret)) > return; > > last = drm_vblank_count(dev, crtc); > @@ -1086,7 +1086,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int > crtc) >last != drm_vblank_count(dev, crtc), >msecs_to_jiffies(100)); > > - WARN_ON(ret == 0); > + WARN(ret == 0, "vblank wait timed out on crtc %i\n", crtc); > > drm_vblank_put(dev, crtc); > } > -- > 2.0.1 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix irq checks in ring->irq_get/put functions
On Mon, Sep 15, 2014 at 02:52:03PM +0200, Daniel Vetter wrote: > Yet another place that wasn't properly transformed when implementing > SOix. While at it convert the checks to WARN_ON on gen5+ (since we > don't have UMS potentially doing stupid things on those platforms). > And also add the corresponding checks to the put functions (again with > a WARN_ON) for gen5+. > > v2: Drop the WARNINGS in the irq_put functions (including the existing > one for vebox), Chris convinced me that they're not that terribly > useful. > > v3: Don't forget about execlist code. > > Cc: Imre Deak > Cc: Jesse Barnes > Cc: "Volkin, Bradley D" > Cc: Chris Wilson > Signed-off-by: Daniel Vetter With the caveat that in my plan I am going to move the check into the single caller of engine->irq_get(), Reviewed-by: Chris Wilson -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 v3 4/4] drm/i915: Support variable cursor height on ivb+
On Sat, Sep 13, 2014 at 05:23:23PM +0100, Chris Wilson wrote: > On Fri, Sep 12, 2014 at 08:53:35PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > IVB introduced the CUR_FBC_CTL register which allows reducing the cursor > > height down to 8 lines from the otherwise square cursor dimensions. > > Implement support for it. > > > > Commandeer the otherwise unused intel_crtc->cursor_size to track the > > current value of CUR_FBC_CTL so that we can avoid duplicating the > > complicated device type checks in i9xx_update_cursor(). > > > > One caveat to note is that CUR_FBC_CTL can't be used with 180 degree > > rotation, so when cursor rotation gets introduced some extra checks are > > needed. > > Where would be a good place to put that note into a comment? I guess I could stick it into cursor_size_ok(). > > So other than the mystery of rotated cursors, the code looks clear > enough. One side question, is the CHV/VLV conflation correct here? Yes. It's still the same old g4x derived display controller. -- 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 v3 3/4] drm/i915: Skip CURBASE write when nothing changed
On Sat, Sep 13, 2014 at 05:20:10PM +0100, Chris Wilson wrote:
> On Fri, Sep 12, 2014 at 08:53:34PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Only write CURBASE when something about the cursor changed. Also
> > eliminate the unnecessary posting read after writing CURCNTR.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 10 ++
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 82c0ad1..60c1aa4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8322,16 +8322,18 @@ static void i9xx_update_cursor(struct drm_crtc
> > *crtc, u32 base)
> > cntl |= CURSOR_PIPE_CSC_ENABLE;
> > }
> >
> > - if (intel_crtc->cursor_cntl != cntl) {
> > + if (intel_crtc->cursor_cntl != cntl)
> > I915_WRITE(CURCNTR(pipe), cntl);
> > - POSTING_READ(CURCNTR(pipe));
> > - intel_crtc->cursor_cntl = cntl;
> > - }
> > +
> > + if (intel_crtc->cursor_cntl == cntl &&
> > + intel_crtc->cursor_base == base)
> > + return;
>
> I'd vote for doing this first and then
>
> I915_WRITE(CURCNTR(pipe), cntl);
>
> unconditionally along with the CURBASE flush.
So you want to write all the cursor registers unconditionally when
anything changes? Would work for CURCNTR, but it would also make
CUR_FBC_CTL stand out like a sore thumb since it can't be written
uncoditionally.
>
> > /* and commit changes on next vblank */
> > I915_WRITE(CURBASE(pipe), base);
> > POSTING_READ(CURBASE(pipe));
> >
> > + intel_crtc->cursor_cntl = cntl;
> > intel_crtc->cursor_base = base;
> > }
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
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 v3 3/4] drm/i915: Skip CURBASE write when nothing changed
On Mon, Sep 15, 2014 at 04:36:38PM +0300, Ville Syrjälä wrote:
> On Sat, Sep 13, 2014 at 05:20:10PM +0100, Chris Wilson wrote:
> > On Fri, Sep 12, 2014 at 08:53:34PM +0300, ville.syrj...@linux.intel.com
> > wrote:
> > > From: Ville Syrjälä
> > >
> > > Only write CURBASE when something about the cursor changed. Also
> > > eliminate the unnecessary posting read after writing CURCNTR.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 10 ++
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 82c0ad1..60c1aa4 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -8322,16 +8322,18 @@ static void i9xx_update_cursor(struct drm_crtc
> > > *crtc, u32 base)
> > > cntl |= CURSOR_PIPE_CSC_ENABLE;
> > > }
> > >
> > > - if (intel_crtc->cursor_cntl != cntl) {
> > > + if (intel_crtc->cursor_cntl != cntl)
> > > I915_WRITE(CURCNTR(pipe), cntl);
> > > - POSTING_READ(CURCNTR(pipe));
> > > - intel_crtc->cursor_cntl = cntl;
> > > - }
> > > +
> > > + if (intel_crtc->cursor_cntl == cntl &&
> > > + intel_crtc->cursor_base == base)
> > > + return;
> >
> > I'd vote for doing this first and then
> >
> > I915_WRITE(CURCNTR(pipe), cntl);
> >
> > unconditionally along with the CURBASE flush.
>
> So you want to write all the cursor registers unconditionally when
> anything changes? Would work for CURCNTR, but it would also make
> CUR_FBC_CTL stand out like a sore thumb since it can't be written
> uncoditionally.
Alternatively, something like
bool dirty = intel_crtc->cursor_base != base;
if (intel_crtc->cursor_cntl != cntl) {
I915_WRITE(CURCNTR(pipe), cntl);
dirty = true;
}
/* ... */
if (!dirty)
return;
I915_WRITE(CURBASE(pipe), base);
POSTING_READ(CURBASE(pipe));
intel_crtc->cursor_cntl = cntl;
intel_crtc->cursor_base = base;
I think would have clearer control flow.
-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/bios: add missing __packed to structs used for reading vbt
This does not seem to make a difference for the structs in question, but
document the intent.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 905999bee2ac..eaaf1ab75554 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -46,7 +46,7 @@ struct bdb_header {
u16 version;/**< decimal */
u16 header_size;/**< in bytes */
u16 bdb_size; /**< in bytes */
-};
+} __packed;
/* strictly speaking, this is a "skip" block, but it has interesting info */
struct vbios_data {
@@ -888,12 +888,12 @@ struct mipi_pps_data {
u16 bl_disable_delay;
u16 panel_off_delay;
u16 panel_power_cycle_delay;
-};
+} __packed;
struct bdb_mipi_config {
struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
-};
+} __packed;
/* Block 53 contains MIPI sequences as needed by the panel
* for enabling it. This block can be variable in size and
@@ -902,7 +902,7 @@ struct bdb_mipi_config {
struct bdb_mipi_sequence {
u8 version;
u8 data[0];
-};
+} __packed;
/* MIPI Sequnece Block definitions */
enum mipi_seq {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915/bios: add missing __packed to structs used for reading vbt
This does not seem to make a difference for the structs in question, but
document the intent.
v2: also pack union child_device_config (Daniel)
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_bios.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index 905999bee2ac..7603765c91fc 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -46,7 +46,7 @@ struct bdb_header {
u16 version;/**< decimal */
u16 header_size;/**< in bytes */
u16 bdb_size; /**< in bytes */
-};
+} __packed;
/* strictly speaking, this is a "skip" block, but it has interesting info */
struct vbios_data {
@@ -252,7 +252,7 @@ union child_device_config {
/* This one should also be safe to use anywhere, even without version
* checks. */
struct common_child_dev_config common;
-};
+} __packed;
struct bdb_general_definitions {
/* DDC GPIO */
@@ -888,12 +888,12 @@ struct mipi_pps_data {
u16 bl_disable_delay;
u16 panel_off_delay;
u16 panel_power_cycle_delay;
-};
+} __packed;
struct bdb_mipi_config {
struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
-};
+} __packed;
/* Block 53 contains MIPI sequences as needed by the panel
* for enabling it. This block can be variable in size and
@@ -902,7 +902,7 @@ struct bdb_mipi_config {
struct bdb_mipi_sequence {
u8 version;
u8 data[0];
-};
+} __packed;
/* MIPI Sequnece Block definitions */
enum mipi_seq {
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-next
Hi Dave, So final feature pull request for 3.18. QA isn't really done yet with the manul testing, but this help up a week of soaking so should be fairly ok-ish. And I think holding up merging doesn't really help anyone, especially since I want to rebase my 3.19 queue on top of drm-next with all the branches that just landed. drm-intel-next-2014-09-05: - final bits (again) for the rotation support (Sonika Jindal) - support bl_power in the intel backlight (Jani) - vdd handling improvements from Ville - i830M fixes from Ville - piles of prep work all over to make skl enabling just plug in (Damien, Sonika) - rename DP training defines to reflect latest edp standards, this touches all drm drivers supporting DP (Sonika Jindal) - cache edids during single detect cycle to avoid re-reading it for e.g. audio, from Chris - move w/a for registers which are stored in the hw context to the context init code (Arun&Damien) - edp panel power sequencer fixes, helps chv a lot (Ville) - piles of other chv fixes all over - much more paranoid pageflip handling with stall detection and better recovery from Chris - small things all over, as usual Aside: A backmerge of latest drm-fixes would be good to baseline 3.19 stuff on top. Note that there's a conflict in i915 which gcc will catch - you need to add a local dev_prive = dev->dev_private somewhere. Cheers, Daniel The following changes since commit 47c1296829505d119d7d58dd23d39cc5db344f12: drm/qxl: enables gem prime helpers for qxl using dummy driver callbacks (2014-09-03 15:36:52 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-next-2014-09-05 for you to fetch changes up to a12624959ad4e3bfa8c344ad71728ffc9a379158: drm/i915: Update DRIVER_DATE to 20140905 (2014-09-05 14:57:29 +0200) - final bits (again) for the rotation support (Sonika Jindal) - support bl_power in the intel backlight (Jani) - vdd handling improvements from Ville - i830M fixes from Ville - piles of prep work all over to make skl enabling just plug in (Damien, Sonika) - rename DP training defines to reflect latest edp standards, this touches all drm drivers supporting DP (Sonika Jindal) - cache edids during single detect cycle to avoid re-reading it for e.g. audio, from Chris - move w/a for registers which are stored in the hw context to the context init code (Arun&Damien) - edp panel power sequencer fixes, helps chv a lot (Ville) - piles of other chv fixes all over - much more paranoid pageflip handling with stall detection and better recovery from Chris - small things all over, as usual Andy Shevchenko (1): drm: i915: reduce memory footprint when debugging Arun Siluvery (2): drm/i915/bdw: Apply workarounds in render ring init function drm/i915/bdw: Export workaround data to debugfs Ben Widawsky (1): drm/i915: Don't save/restore RS when not used Chris Wilson (15): drm/i915: Do not access stolen memory directly by the CPU, even for error capture drm/i915: Remove num_pages parameter to i915_error_object_create() drm/i915: Suppress a WARN on reading an object back for a GPU hang drm/i915: honour forced connector modes drm/i915: Make wait-for-pending-flips more defensive drm/i915: Differentiate between LLC or snooped for the user drm/i915/dp: Refactor common eDP lid detection drm/i915/dp: Cache EDID for a detection cycle drm/i915/hdmi: Cache EDID for a detection cycle drm/i915: Rename global latency_ns variable drm/i915: Remove shadowed local variable 'i' from i915_interrupt_info drm/i915: Fix unsafe vma iteration in i915_drop_caches drm/i915: Reset the HEAD pointer for the ring after writing START drm/i915: Check for a stalled page flip after each vblank drm/i915: Decouple the stuck pageflip on modeset Daisy Sun (1): drm/i915/bdw: BDW Software Turbo Damien Lespiau (12): drm/i915: Use dev_priv as first argument of for_each_pipe() drm/i915: Print the pipe on which the vblank wait times out drm/i915: Don't use a define when it's clearer to just put the value drm/i915: Add "Intel Corporation" as module author drm/i915/bdw: Let the memory controller do all the swizzling drm/i915: Rename intel_wa_registers with a i915_ prefix drm/i915: Don't overrun the intel_wa_regs array drm/i915: Don't silently discard workarounds drm/i915: Remove unneeded brackets drm/i915: Don't restrict i915_wa_registers to BDW drm/i915: Rewrite ABS_DIFF() in a safer manner drm/i915: Introduce a for_each_plane() macro Daniel Vetter (2): MAINTAINERS: Update Daniel Vetter's email address drm/i915: Update DRIVER_DATE to 20140905 Deepak S (2): drm/i915: Bring UP Power Wells before disabling RC6. drm/i915: Fix to Enab
Re: [Intel-gfx] [PATCH v2] drm/i915/bios: add missing __packed to structs used for reading vbt
On Mon, Sep 15, 2014 at 04:59:28PM +0300, Jani Nikula wrote:
> This does not seem to make a difference for the structs in question, but
> document the intent.
>
> v2: also pack union child_device_config (Daniel)
>
> Signed-off-by: Jani Nikula
Queued for -next, thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_bios.h | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 905999bee2ac..7603765c91fc 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -46,7 +46,7 @@ struct bdb_header {
> u16 version;/**< decimal */
> u16 header_size;/**< in bytes */
> u16 bdb_size; /**< in bytes */
> -};
> +} __packed;
>
> /* strictly speaking, this is a "skip" block, but it has interesting info */
> struct vbios_data {
> @@ -252,7 +252,7 @@ union child_device_config {
> /* This one should also be safe to use anywhere, even without version
>* checks. */
> struct common_child_dev_config common;
> -};
> +} __packed;
>
> struct bdb_general_definitions {
> /* DDC GPIO */
> @@ -888,12 +888,12 @@ struct mipi_pps_data {
> u16 bl_disable_delay;
> u16 panel_off_delay;
> u16 panel_power_cycle_delay;
> -};
> +} __packed;
>
> struct bdb_mipi_config {
> struct mipi_config config[MAX_MIPI_CONFIGURATIONS];
> struct mipi_pps_data pps[MAX_MIPI_CONFIGURATIONS];
> -};
> +} __packed;
>
> /* Block 53 contains MIPI sequences as needed by the panel
> * for enabling it. This block can be variable in size and
> @@ -902,7 +902,7 @@ struct bdb_mipi_config {
> struct bdb_mipi_sequence {
> u8 version;
> u8 data[0];
> -};
> +} __packed;
>
> /* MIPI Sequnece Block definitions */
> enum mipi_seq {
> --
> 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
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend GET_APERTURE ioctl to report available map space
On Mon, Sep 15, 2014 at 01:33:44PM +0100, Chris Wilson wrote:
> When constructing a batchbuffer, it is sometimes crucial to know the
> largest hole into which we can fit a fenceable buffer (for example when
> handling very large objects on gen2 and gen3). This depends on the
> fragmentation of pinned buffers inside the aperture, a question only the
> kernel can easily answer.
>
> This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
> include a couple of new fields in its reply to userspace - the total
> amount of space available in the mappable region of the aperture and
> also the single largest block available.
>
> This is not quite what userspace wants to answer the question of whether
> this batch will fit as fences are also required to meet severe alignment
> constraints within the batch. For this purpose, a third conservative
> estimate of largest fence available is also provided. For when userspace
> needs more than one batch, we also provide the culmulative space
> available for fences such that it has some additional guidance to how
> much space it could allocate to fences. Conservatism still wins.
>
> The patch also adds a debugfs file for convenient testing and reporting.
>
> Signed-off-by: Chris Wilson
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 28 +
> drivers/gpu/drm/i915/i915_gem.c | 111
> ++--
> include/uapi/drm/i915_drm.h | 20 +++
> 3 files changed, 155 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e63ccbea52e..41d92f29aef1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -534,6 +534,33 @@ static int obj_rank_by_ggtt(void *priv, struct list_head
> *A, struct list_head *B
> return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
> }
>
> +static int i915_gem_aperture_info(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct drm_i915_gem_get_aperture arg;
> + int ret;
> +
> + ret = i915_gem_get_aperture_ioctl(dev, &arg, NULL);
> + if (ret)
> + return ret;
> +
> + seq_printf(m, "Total size of the GTT: %llu bytes\n",
> +arg.aper_size);
> + seq_printf(m, "Available space in the GTT: %llu bytes\n",
> +arg.aper_available_size);
> + seq_printf(m, "Available space in the mappable aperture: %u bytes\n",
> +arg.map_available_size);
> + seq_printf(m, "Single largest space in the mappable aperture: %u
> bytes\n",
> +arg.map_largest_size);
> + seq_printf(m, "Available space for fences: %u bytes\n",
> +arg.fence_available_size);
> + seq_printf(m, "Single largest fence available: %u bytes\n",
> +arg.fence_largest_size);
> +
> + return 0;
> +}
> +
> static int i915_gem_gtt_info(struct seq_file *m, void *data)
> {
> struct drm_info_node *node = m->private;
> @@ -4198,6 +4225,7 @@ static int i915_debugfs_create(struct dentry *root,
> static const struct drm_info_list i915_debugfs_list[] = {
> {"i915_capabilities", i915_capabilities, 0},
> {"i915_gem_objects", i915_gem_object_info, 0},
> + {"i915_gem_aperture", i915_gem_aperture_info, 0},
> {"i915_gem_gtt", i915_gem_gtt_info, 0},
> {"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
> {"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4b9de297b967..4b75086a1dc9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -31,6 +31,7 @@
> #include "i915_drv.h"
> #include "i915_trace.h"
> #include "intel_drv.h"
> +#include
> #include
> #include
> #include
> @@ -260,6 +261,49 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data,
> return 0;
> }
>
> +static int obj_rank_by_ggtt(void *priv,
> + struct list_head *A,
> + struct list_head *B)
> +{
> + struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
> + struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
> +
> + return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
> +}
> +
> +static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32
> end)
> +{
> + u32 size = end - start;
> + u32 fence_size;
> +
> + if (INTEL_INFO(dev_priv)->gen < 4) {
> + u32 fence_max;
> + u32 fence_next;
> +
> + if (IS_GEN3(dev_priv)) {
> + fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
> + fence_next = 1024*1024;
> + } else {
> + fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
> + fence_ne
[Intel-gfx] [PATCH 06/11] drm/i915: Clarify irq_lock locking, interrupt install/uninstall
All the interrupt setup/teardown hooks are always run from plain
process context. So again just the _irq variant is good enough.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_irq.c | 42 ++---
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4906823baa11..a829619aa111 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3603,7 +3603,6 @@ static void gen5_gt_irq_postinstall(struct drm_device
*dev)
static int ironlake_irq_postinstall(struct drm_device *dev)
{
- unsigned long irqflags;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 display_mask, extra_mask;
@@ -3642,9 +3641,9 @@ static int ironlake_irq_postinstall(struct drm_device
*dev)
* spinlocking not required here for correctness since interrupt
* setup is guaranteed to run in single-threaded context. But we
* need it to make the assert_spin_locked happy. */
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
return 0;
@@ -3740,7 +3739,6 @@ void valleyview_disable_display_irqs(struct
drm_i915_private *dev_priv)
static int valleyview_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long irqflags;
dev_priv->irq_mask = ~0;
@@ -3754,10 +3752,10 @@ static int valleyview_irq_postinstall(struct drm_device
*dev)
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display_irqs_enabled)
valleyview_display_irqs_install(dev_priv);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
I915_WRITE(VLV_IIR, 0x);
I915_WRITE(VLV_IIR, 0x);
@@ -3848,7 +3846,6 @@ static int cherryview_irq_postinstall(struct drm_device
*dev)
I915_DISPLAY_PIPE_C_EVENT_INTERRUPT;
u32 pipestat_enable = PLANE_FLIP_DONE_INT_STATUS_VLV |
PIPE_CRC_DONE_INTERRUPT_STATUS;
- unsigned long irqflags;
int pipe;
/*
@@ -3860,11 +3857,11 @@ static int cherryview_irq_postinstall(struct drm_device
*dev)
for_each_pipe(dev_priv, pipe)
I915_WRITE(PIPESTAT(pipe), 0x);
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
i915_enable_pipestat(dev_priv, PIPE_A, PIPE_GMBUS_INTERRUPT_STATUS);
for_each_pipe(dev_priv, pipe)
i915_enable_pipestat(dev_priv, pipe, pipestat_enable);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
I915_WRITE(VLV_IIR, 0x);
I915_WRITE(VLV_IMR, dev_priv->irq_mask);
@@ -3891,7 +3888,6 @@ static void gen8_irq_uninstall(struct drm_device *dev)
static void valleyview_irq_uninstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long irqflags;
int pipe;
if (!dev_priv)
@@ -3906,10 +3902,12 @@ static void valleyview_irq_uninstall(struct drm_device
*dev)
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ /* Interrupt setup is already guaranteed to be single-threaded, this is
+* just to make the assert_spin_locked check happy. */
+ spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display_irqs_enabled)
valleyview_display_irqs_uninstall(dev_priv);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
dev_priv->irq_mask = 0;
@@ -3995,7 +3993,6 @@ static void i8xx_irq_preinstall(struct drm_device * dev)
static int i8xx_irq_postinstall(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long irqflags;
I915_WRITE16(EMR,
~(I915_ERROR_PAGE_TABLE | I915_ERROR_MEMORY_REFRESH));
@@ -4018,10 +4015,10 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
/* Interrupt setup is already guaranteed to be single-threaded, this is
* just to make the assert_spin_locked check happy. */
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
i915_enable_pipestat(dev_
[Intel-gfx] [PATCH 05/11] drm/i915: Clarify irq_lock locking, work functions
Work functions are in process context, so plain _irq spinlock variants
is all we need.
The hpd reenable work didn't follow the _work/_work_func postfix
naming scheme, so adjust that while at it.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_irq.c | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d22f87020aee..4906823baa11 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1096,18 +1096,17 @@ static void i915_digport_work_func(struct work_struct
*work)
{
struct drm_i915_private *dev_priv =
container_of(work, struct drm_i915_private, dig_port_work);
- unsigned long irqflags;
u32 long_port_mask, short_port_mask;
struct intel_digital_port *intel_dig_port;
int i, ret;
u32 old_bits = 0;
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
long_port_mask = dev_priv->long_hpd_port_mask;
dev_priv->long_hpd_port_mask = 0;
short_port_mask = dev_priv->short_hpd_port_mask;
dev_priv->short_hpd_port_mask = 0;
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
for (i = 0; i < I915_MAX_PORTS; i++) {
bool valid = false;
@@ -1132,9 +1131,9 @@ static void i915_digport_work_func(struct work_struct
*work)
}
if (old_bits) {
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
dev_priv->hpd_event_bits |= old_bits;
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
schedule_work(&dev_priv->hotplug_work);
}
}
@@ -1153,7 +1152,6 @@ static void i915_hotplug_work_func(struct work_struct
*work)
struct intel_connector *intel_connector;
struct intel_encoder *intel_encoder;
struct drm_connector *connector;
- unsigned long irqflags;
bool hpd_disabled = false;
bool changed = false;
u32 hpd_event_bits;
@@ -1161,7 +1159,7 @@ static void i915_hotplug_work_func(struct work_struct
*work)
mutex_lock(&mode_config->mutex);
DRM_DEBUG_KMS("running encoder hotplug functions\n");
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
hpd_event_bits = dev_priv->hpd_event_bits;
dev_priv->hpd_event_bits = 0;
@@ -1195,7 +1193,7 @@ static void i915_hotplug_work_func(struct work_struct
*work)
msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
}
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
list_for_each_entry(connector, &mode_config->connector_list, head) {
intel_connector = to_intel_connector(connector);
@@ -1490,7 +1488,6 @@ static void ivybridge_parity_work(struct work_struct
*work)
u32 error_status, row, bank, subbank;
char *parity_event[6];
uint32_t misccpctl;
- unsigned long flags;
uint8_t slice = 0;
/* We must turn off DOP level clock gating to access the L3 registers.
@@ -1549,9 +1546,9 @@ static void ivybridge_parity_work(struct work_struct
*work)
out:
WARN_ON(dev_priv->l3_parity.which_slice);
- spin_lock_irqsave(&dev_priv->irq_lock, flags);
+ spin_lock_irq(&dev_priv->irq_lock);
gen5_enable_gt_irq(dev_priv, GT_PARITY_ERROR(dev_priv->dev));
- spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+ spin_unlock_irq(&dev_priv->irq_lock);
mutex_unlock(&dev_priv->dev->struct_mutex);
}
@@ -4606,19 +4603,18 @@ static void i965_irq_uninstall(struct drm_device * dev)
I915_WRITE(IIR, I915_READ(IIR));
}
-static void intel_hpd_irq_reenable(struct work_struct *work)
+static void intel_hpd_irq_reenable_work(struct work_struct *work)
{
struct drm_i915_private *dev_priv =
container_of(work, typeof(*dev_priv),
hotplug_reenable_work.work);
struct drm_device *dev = dev_priv->dev;
struct drm_mode_config *mode_config = &dev->mode_config;
- unsigned long irqflags;
int i;
intel_runtime_pm_get(dev_priv);
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
struct drm_connector *connector;
@@ -4642,7 +4638,7 @@ static void intel_hpd_irq_reenable(struct work_struct
*work)
}
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
[Intel-gfx] [PATCH 02/11] drm/i915: Clarify event_lock locking, irq&mixed context
Now we tackle the functions also called from interrupt handlers.
- intel_check_page_flip is exclusively called from irq handlers, so a
plain spin_lock is all we need. In i915_irq.c we have the convention
to give all such functions an _irq_handler postfix, but that would
look strange and als be a bit a misleading name. I've opted for a
WARN_ON(!in_irq()) instead.
- The other two places left are called both from interrupt handlers
and from our reset work, so need the full irqsave dance. Annotate
them with a short comment.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index dcbefe510a2a..d120dfff3841 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9385,6 +9385,10 @@ static void do_intel_finish_page_flip(struct drm_device
*dev,
if (intel_crtc == NULL)
return;
+ /*
+* This is called both by irq handlers and the reset code (to complete
+* lost pageflips) so needs the full irqsave spinlocks.
+*/
spin_lock_irqsave(&dev->event_lock, flags);
work = intel_crtc->unpin_work;
@@ -9466,7 +9470,12 @@ void intel_prepare_page_flip(struct drm_device *dev, int
plane)
to_intel_crtc(dev_priv->plane_to_crtc_mapping[plane]);
unsigned long flags;
- /* NB: An MMIO update of the plane base pointer will also
+
+ /*
+* This is called both by irq handlers and the reset code (to complete
+* lost pageflips) so needs the full irqsave spinlocks.
+*
+* NB: An MMIO update of the plane base pointer will also
* generate a page-flip completion irq, i.e. every modeset
* is also accompanied by a spurious intel_prepare_page_flip().
*/
@@ -9923,18 +9932,19 @@ void intel_check_page_flip(struct drm_device *dev, int
pipe)
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- unsigned long flags;
+
+ WARN_ON(!in_irq());
if (crtc == NULL)
return;
- spin_lock_irqsave(&dev->event_lock, flags);
+ spin_lock(&dev->event_lock);
if (intel_crtc->unpin_work && __intel_pageflip_stall_check(dev, crtc)) {
WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
intel_crtc->unpin_work->flip_queued_vblank,
drm_vblank_count(dev, pipe));
page_flip_completed(intel_crtc);
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ spin_unlock(&dev->event_lock);
}
static int intel_crtc_page_flip(struct drm_crtc *crtc,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/11] Spinlock use clarification in i915
Hi all, So I've tried to figure out a way how to clear up our irq setup mess, but instead only managed to get sidetracked on a spinlock usage review. Review highly welcome. Cheers, Daniel Daniel Vetter (11): drm/i915: Clarify event_lock locking, process context drm/i915: Clarify event_lock locking, irq&mixed context drm/i915: Clarify gpu_error.lock locking drm/i915: Clarify irq_lock locking, intel_tv_detect drm/i915: Clarify irq_lock locking, work functions drm/i915: Clarify irq_lock locking, interrupt install/uninstall drm/i915: Clarify irq_lock locking, irq handlers drm/i915: Clarify irq_lock locking, special cases drm/i915: Convert backlight_lock to a mutex drm/i915: Clarify uncore.lock locking drm/i915: Clarify mmio_flip_lock locking drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 5 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gpu_error.c | 10 ++-- drivers/gpu/drm/i915/i915_irq.c | 101 ++ drivers/gpu/drm/i915/intel_display.c | 67 +++--- drivers/gpu/drm/i915/intel_panel.c| 30 -- drivers/gpu/drm/i915/intel_tv.c | 9 ++- 9 files changed, 103 insertions(+), 128 deletions(-) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/11] drm/i915: Clarify irq_lock locking, irq handlers
irq handlers always run with interrupts locally disabled, so plain spinlocks is all we need. I've also reviewed again that they all follow the _irq_handler postfix convention. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_irq.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index a829619aa111..6a4f389ff2f5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4063,7 +4063,6 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) struct drm_i915_private *dev_priv = dev->dev_private; u16 iir, new_iir; u32 pipe_stats[2]; - unsigned long irqflags; int pipe; u16 flip_mask = I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | @@ -4079,7 +4078,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) * It doesn't set the bit in iir again, but it still produces * interrupts (for non-MSI). */ - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + spin_lock(&dev_priv->irq_lock); if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT) i915_handle_error(dev, false, "Command parser error, iir 0x%08x", @@ -4095,7 +4094,7 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg) if (pipe_stats[pipe] & 0x8000) I915_WRITE(reg, pipe_stats[pipe]); } - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + spin_unlock(&dev_priv->irq_lock); I915_WRITE16(IIR, iir & ~flip_mask); new_iir = I915_READ16(IIR); /* Flush posted writes */ @@ -4249,7 +4248,6 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) struct drm_device *dev = arg; struct drm_i915_private *dev_priv = dev->dev_private; u32 iir, new_iir, pipe_stats[I915_MAX_PIPES]; - unsigned long irqflags; u32 flip_mask = I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT; @@ -4265,7 +4263,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) * It doesn't set the bit in iir again, but it still produces * interrupts (for non-MSI). */ - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + spin_lock(&dev_priv->irq_lock); if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT) i915_handle_error(dev, false, "Command parser error, iir 0x%08x", @@ -4281,7 +4279,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg) irq_received = true; } } - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + spin_unlock(&dev_priv->irq_lock); if (!irq_received) break; @@ -4476,7 +4474,6 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) struct drm_i915_private *dev_priv = dev->dev_private; u32 iir, new_iir; u32 pipe_stats[I915_MAX_PIPES]; - unsigned long irqflags; int ret = IRQ_NONE, pipe; u32 flip_mask = I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT | @@ -4493,7 +4490,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) * It doesn't set the bit in iir again, but it still produces * interrupts (for non-MSI). */ - spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + spin_lock(&dev_priv->irq_lock); if (iir & I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT) i915_handle_error(dev, false, "Command parser error, iir 0x%08x", @@ -4511,7 +4508,7 @@ static irqreturn_t i965_irq_handler(int irq, void *arg) irq_received = true; } } - spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); + spin_unlock(&dev_priv->irq_lock); if (!irq_received) break; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/11] drm/i915: Clarify irq_lock locking, special cases
Grab bag for all the special cases:
- i9xx_check_fifo_underruns is only called from crtc_enable hooks,
i.e. process context.
- i915_enable_asle_pipestat is only called from interrupt postinstall
hooks. So again process context.
- gen8_irq_power_well_post_enable is called from the runtime pm code,
which again means process context.
- The open-coded hpd_irq_setup loop in _thaw is also running in process
context.
So for all of them the plain _irq variant is sufficient.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_drv.c | 5 ++---
drivers/gpu/drm/i915/i915_irq.c | 16 ++--
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b8bd0080603e..8ce1b13ad97e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -686,11 +686,10 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
intel_modeset_init_hw(dev);
{
- unsigned long irqflags;
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
intel_dp_mst_resume(dev);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6a4f389ff2f5..a08cdc62f841 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -310,9 +310,8 @@ void i9xx_check_fifo_underruns(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *crtc;
- unsigned long flags;
- spin_lock_irqsave(&dev_priv->irq_lock, flags);
+ spin_lock_irq(&dev_priv->irq_lock);
for_each_intel_crtc(dev, crtc) {
u32 reg = PIPESTAT(crtc->pipe);
@@ -331,7 +330,7 @@ void i9xx_check_fifo_underruns(struct drm_device *dev)
DRM_ERROR("pipe %c underrun\n", pipe_name(crtc->pipe));
}
- spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
static void i9xx_set_fifo_underrun_reporting(struct drm_device *dev,
@@ -696,19 +695,18 @@ i915_disable_pipestat(struct drm_i915_private *dev_priv,
enum pipe pipe,
static void i915_enable_asle_pipestat(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long irqflags;
if (!dev_priv->opregion.asle || !IS_MOBILE(dev))
return;
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
i915_enable_pipestat(dev_priv, PIPE_B, PIPE_LEGACY_BLC_EVENT_STATUS);
if (INTEL_INFO(dev)->gen >= 4)
i915_enable_pipestat(dev_priv, PIPE_A,
PIPE_LEGACY_BLC_EVENT_STATUS);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
/**
@@ -3477,14 +3475,12 @@ static void gen8_irq_reset(struct drm_device *dev)
void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv)
{
- unsigned long irqflags;
-
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_B, dev_priv->de_irq_mask[PIPE_B],
~dev_priv->de_irq_mask[PIPE_B]);
GEN8_IRQ_INIT_NDX(DE_PIPE, PIPE_C, dev_priv->de_irq_mask[PIPE_C],
~dev_priv->de_irq_mask[PIPE_C]);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
static void cherryview_irq_preinstall(struct drm_device *dev)
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/11] drm/i915: Clarify uncore.lock locking
Only one place looked in need of a bit of polish: hsw_restore_lcpll.
It's used by the runtime pm code and hence is always called from
process context. No irq flag saving required.
Another thing I've stumbled over is that we might need to add a
raw forcewake_get/put helpers which don't grab a runtime pm reference
but just check that the device isn't suspended - we have this duplicated
in the execlist code, too.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d120dfff3841..c01d783e59b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7660,7 +7660,6 @@ static void hsw_disable_lcpll(struct drm_i915_private
*dev_priv,
static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
{
uint32_t val;
- unsigned long irqflags;
val = I915_READ(LCPLL_CTL);
@@ -7680,10 +7679,10 @@ static void hsw_restore_lcpll(struct drm_i915_private
*dev_priv)
* to call special forcewake code that doesn't touch runtime PM and
* doesn't enable the forcewake delayed work.
*/
- spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+ spin_lock_irq(&dev_priv->uncore.lock);
if (dev_priv->uncore.forcewake_count++ == 0)
dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
- spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+ spin_unlock_irq(&dev_priv->uncore.lock);
if (val & LCPLL_POWER_DOWN_ALLOW) {
val &= ~LCPLL_POWER_DOWN_ALLOW;
@@ -7714,10 +7713,10 @@ static void hsw_restore_lcpll(struct drm_i915_private
*dev_priv)
}
/* See the big comment above. */
- spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
+ spin_lock_irq(&dev_priv->uncore.lock);
if (--dev_priv->uncore.forcewake_count == 0)
dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
- spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+ spin_unlock_irq(&dev_priv->uncore.lock);
}
/*
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/11] drm/i915: Clarify event_lock locking, process context
It's good practice to use the more specific versions for irq save
spinlocks both as executable documentation and to enforce saner
design. The _irqsave version really should only be used if the calling
context is unknown and there's a good reason to call a function from
all kinds of places.
This is the first step whice replaces all occurances of _irqsave in
process context with the simpler irq disable/enable variants. We don't
have any funky spinlock nesting going on, especially since the
event_lock is the outermost of the irq/vblank related spinlocks.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 ++---
drivers/gpu/drm/i915/intel_display.c | 35 +++
2 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 063b44817e08..0ba5c7145240 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -516,7 +516,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
*data)
struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long flags;
struct intel_crtc *crtc;
int ret;
@@ -529,7 +528,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
*data)
const char plane = plane_name(crtc->plane);
struct intel_unpin_work *work;
- spin_lock_irqsave(&dev->event_lock, flags);
+ spin_lock_irq(&dev->event_lock);
work = crtc->unpin_work;
if (work == NULL) {
seq_printf(m, "No flip due on pipe %c (plane %c)\n",
@@ -575,7 +574,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
*data)
seq_printf(m, "MMIO update completed? %d\n",
addr == work->gtt_offset);
}
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ spin_unlock_irq(&dev->event_lock);
}
mutex_unlock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 82c0ad1f6a2e..dcbefe510a2a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2765,16 +2765,15 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
*crtc)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- unsigned long flags;
bool pending;
if (i915_reset_in_progress(&dev_priv->gpu_error) ||
intel_crtc->reset_counter !=
atomic_read(&dev_priv->gpu_error.reset_counter))
return false;
- spin_lock_irqsave(&dev->event_lock, flags);
+ spin_lock_irq(&dev->event_lock);
pending = to_intel_crtc(crtc)->unpin_work != NULL;
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ spin_unlock_irq(&dev->event_lock);
return pending;
}
@@ -3485,14 +3484,13 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc
*crtc)
!intel_crtc_has_pending_flip(crtc),
60*HZ) == 0)) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- unsigned long flags;
- spin_lock_irqsave(&dev->event_lock, flags);
+ spin_lock_irq(&dev->event_lock);
if (intel_crtc->unpin_work) {
WARN_ONCE(1, "Removing stuck page flip\n");
page_flip_completed(intel_crtc);
}
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ spin_unlock_irq(&dev->event_lock);
}
if (crtc->primary->fb) {
@@ -9337,12 +9335,11 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct intel_unpin_work *work;
- unsigned long flags;
- spin_lock_irqsave(&dev->event_lock, flags);
+ spin_lock_irq(&dev->event_lock);
work = intel_crtc->unpin_work;
intel_crtc->unpin_work = NULL;
- spin_unlock_irqrestore(&dev->event_lock, flags);
+ spin_unlock_irq(&dev->event_lock);
if (work) {
cancel_work_sync(&work->work);
@@ -9953,7 +9950,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe;
struct intel_unpin_work *work;
struct intel_engine_cs *ring;
- unsigned long flags;
int ret;
//trigger software GT busyness calculation
@@ -9997,7 +9993,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
goto free_work;
/* We borrow the event spin lock for
[Intel-gfx] [PATCH 03/11] drm/i915: Clarify gpu_error.lock locking
i915_capture_error_state can be called from all kinds of contexts, so
needs the full irqsave dance. But the other two places to grab and
release the error state are only called from process context. So
simplify them to the plaine _irq spinlock versions to clarify the
locking semantics.
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gpu_error.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2c87a797213f..386e45dbeff1 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1326,13 +1326,12 @@ void i915_error_state_get(struct drm_device *dev,
struct i915_error_state_file_priv *error_priv)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long flags;
- spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ spin_lock_irq(&dev_priv->gpu_error.lock);
error_priv->error = dev_priv->gpu_error.first_error;
if (error_priv->error)
kref_get(&error_priv->error->ref);
- spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
+ spin_unlock_irq(&dev_priv->gpu_error.lock);
}
@@ -1346,12 +1345,11 @@ void i915_destroy_error_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_i915_error_state *error;
- unsigned long flags;
- spin_lock_irqsave(&dev_priv->gpu_error.lock, flags);
+ spin_lock_irq(&dev_priv->gpu_error.lock);
error = dev_priv->gpu_error.first_error;
dev_priv->gpu_error.first_error = NULL;
- spin_unlock_irqrestore(&dev_priv->gpu_error.lock, flags);
+ spin_unlock_irq(&dev_priv->gpu_error.lock);
if (error)
kref_put(&error->ref, i915_error_state_free);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 11/11] drm/i915: Clarify mmio_flip_lock locking
The ->queue_flip callback is always called from process context, so
plain _irq spinlock variants are enough.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_display.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c01d783e59b1..a8632f6937ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9849,7 +9849,6 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- unsigned long irq_flags;
int ret;
if (WARN_ON(intel_crtc->mmio_flip.seqno))
@@ -9863,10 +9862,10 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
return 0;
}
- spin_lock_irqsave(&dev_priv->mmio_flip_lock, irq_flags);
+ spin_lock_irq(&dev_priv->mmio_flip_lock);
intel_crtc->mmio_flip.seqno = obj->last_write_seqno;
intel_crtc->mmio_flip.ring_id = obj->ring->id;
- spin_unlock_irqrestore(&dev_priv->mmio_flip_lock, irq_flags);
+ spin_unlock_irq(&dev_priv->mmio_flip_lock);
/*
* Double check to catch cases where irq fired before
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/11] drm/i915: Clarify irq_lock locking, intel_tv_detect
->detect callbacks are only ever called from process context, and
there's no fancy nesting going on here. So plain _irq spinlock
variants is what we want.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/intel_tv.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index c14341ca3ef9..6f5f59b880f5 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1182,18 +1182,17 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- unsigned long irqflags;
u32 tv_ctl, save_tv_ctl;
u32 tv_dac, save_tv_dac;
int type;
/* Disable TV interrupts around load detect or we'll recurse */
if (connector->polled & DRM_CONNECTOR_POLL_HPD) {
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
i915_disable_pipestat(dev_priv, 0,
PIPE_HOTPLUG_INTERRUPT_STATUS |
PIPE_HOTPLUG_TV_INTERRUPT_STATUS);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
save_tv_dac = tv_dac = I915_READ(TV_DAC);
@@ -1266,11 +1265,11 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
/* Restore interrupt config */
if (connector->polled & DRM_CONNECTOR_POLL_HPD) {
- spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ spin_lock_irq(&dev_priv->irq_lock);
i915_enable_pipestat(dev_priv, 0,
PIPE_HOTPLUG_INTERRUPT_STATUS |
PIPE_HOTPLUG_TV_INTERRUPT_STATUS);
- spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ spin_unlock_irq(&dev_priv->irq_lock);
}
return type;
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/11] drm/i915: Convert backlight_lock to a mutex
Originally the irq safe spinlock was required because of asle
interrupts. But since
commit 7bd688cd66db93f6430f6e2b3145ee5686daa315
Author: Jani Nikula
Date: Fri Nov 8 16:48:56 2013 +0200
drm/i915: handle backlight through chip specific functions
there's no need for this any more. So switch to the simpler mutex.
Cc: Jani Nikula
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/i915/i915_dma.c| 2 +-
drivers/gpu/drm/i915/i915_drv.h| 2 +-
drivers/gpu/drm/i915/intel_panel.c | 30 --
3 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1403b01e8216..0bc1583114e7 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1614,7 +1614,7 @@ int i915_driver_load(struct drm_device *dev, unsigned
long flags)
spin_lock_init(&dev_priv->irq_lock);
spin_lock_init(&dev_priv->gpu_error.lock);
- spin_lock_init(&dev_priv->backlight_lock);
+ mutex_init(&dev_priv->backlight_lock);
spin_lock_init(&dev_priv->uncore.lock);
spin_lock_init(&dev_priv->mm.object_stat_lock);
spin_lock_init(&dev_priv->mmio_flip_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17dfce0f4e68..07dafa2c2d8c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1528,7 +1528,7 @@ struct drm_i915_private {
struct intel_overlay *overlay;
/* backlight registers and fields in struct intel_panel */
- spinlock_t backlight_lock;
+ struct mutex backlight_lock;
/* LVDS info */
bool no_aux_handshake;
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 18784470a760..f17ada3742de 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -538,14 +538,13 @@ static u32 intel_panel_get_backlight(struct
intel_connector *connector)
struct drm_device *dev = connector->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
u32 val;
- unsigned long flags;
- spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+ mutex_lock(&dev_priv->backlight_lock);
val = dev_priv->display.get_backlight(connector);
val = intel_panel_compute_brightness(connector, val);
- spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+ mutex_unlock(&dev_priv->backlight_lock);
DRM_DEBUG_DRIVER("get backlight PWM = %d\n", val);
return val;
@@ -629,12 +628,11 @@ static void intel_panel_set_backlight(struct
intel_connector *connector,
struct intel_panel *panel = &connector->panel;
enum pipe pipe = intel_get_pipe_from_connector(connector);
u32 hw_level;
- unsigned long flags;
if (!panel->backlight.present || pipe == INVALID_PIPE)
return;
- spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+ mutex_lock(&dev_priv->backlight_lock);
WARN_ON(panel->backlight.max == 0);
@@ -644,7 +642,7 @@ static void intel_panel_set_backlight(struct
intel_connector *connector,
if (panel->backlight.enabled)
intel_panel_actually_set_backlight(connector, hw_level);
- spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+ mutex_unlock(&dev_priv->backlight_lock);
}
/* set backlight brightness to level in range [0..max], assuming hw min is
@@ -658,12 +656,11 @@ void intel_panel_set_backlight_acpi(struct
intel_connector *connector,
struct intel_panel *panel = &connector->panel;
enum pipe pipe = intel_get_pipe_from_connector(connector);
u32 hw_level;
- unsigned long flags;
if (!panel->backlight.present || pipe == INVALID_PIPE)
return;
- spin_lock_irqsave(&dev_priv->backlight_lock, flags);
+ mutex_lock(&dev_priv->backlight_lock);
WARN_ON(panel->backlight.max == 0);
@@ -679,7 +676,7 @@ void intel_panel_set_backlight_acpi(struct intel_connector
*connector,
if (panel->backlight.enabled)
intel_panel_actually_set_backlight(connector, hw_level);
- spin_unlock_irqrestore(&dev_priv->backlight_lock, flags);
+ mutex_unlock(&dev_priv->backlight_lock);
}
static void pch_disable_backlight(struct intel_connector *connector)
@@ -733,7 +730,6 @@ void intel_panel_disable_backlight(struct intel_connector
*connector)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_panel *panel = &connector->panel;
enum pipe pipe = intel_get_pipe_from_connector(connector);
- unsigned long flags;
if (!panel->backlight.present || pipe == INVALID_PIPE)
return;
@@ -749,14 +745,14 @@ void intel_panel_disable_backlight(struct intel_connector
*connector)
return;
}
- spin_lock_irqsave(&dev_priv->backlight_loc
[Intel-gfx] [PATCH v2 16/16] drm/i915: add comments on what stage a given PM handler is called
This will hopefully make it easier to navigate the code without the need
to consult the full PM documentation.
v2:
- add a comment that the freeze handler is also called after rebooting
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b5508ae..538a658 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1504,10 +1504,24 @@ static int intel_resume_prepare(struct drm_i915_private
*dev_priv,
}
static const struct dev_pm_ops i915_pm_ops = {
+ /* S0ix, S3 event handlers */
.suspend = i915_pm_suspend,
.suspend_late = i915_pm_suspend_late,
.resume_early = i915_pm_resume_early,
.resume = i915_pm_resume,
+
+ /*
+* S4 event handlers
+* @freeze, @freeze_late: called (1) before creating hibernation
+*image and (2) after rebooting, before
+*restoring the image
+* @thaw, @thaw_early : called after creating hibernation image,
+*before writing it
+* @poweroff, @poweroff_late: called after writing hibernation image,
+*before rebooting
+* @restore, @restore_early : called after rebooting and restoring the
+*image
+*/
.freeze = i915_pm_suspend,
.freeze_late = i915_pm_suspend_late,
.thaw_early = i915_pm_resume_early,
@@ -1516,6 +1530,8 @@ static const struct dev_pm_ops i915_pm_ops = {
.poweroff_late = i915_pm_suspend_late,
.restore_early = i915_pm_resume_early,
.restore = i915_pm_resume,
+
+ /* D0<->D3 (runtime PM) event handlers */
.runtime_suspend = intel_runtime_suspend,
.runtime_resume = intel_runtime_resume,
};
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 09/16] drm/i915: check for GT faults in all resume handlers and driver load time
Checking for GT faults is not specific in any way to S4 thaw, so do it
also during S3 resume, S4 restore and driver load time. This allows us to
unify the Sx handlers in an upcoming patch.
v2:
- move the check to intel_uncore_early_sanitize(), so we check at driver
load time too (Chris)
Signed-off-by: Imre Deak
---
drivers/gpu/drm/i915/i915_drv.c | 3 ---
drivers/gpu/drm/i915/intel_uncore.c | 13 +++--
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5e25283..3090a94 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -736,9 +736,6 @@ static int __i915_drm_thaw(struct drm_device *dev)
static int i915_drm_thaw(struct drm_device *dev)
{
- if (drm_core_check_feature(dev, DRIVER_MODESET))
- i915_check_and_clear_faults(dev);
-
return __i915_drm_thaw(dev);
}
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 918b761..cc46ac5 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -363,7 +363,8 @@ void intel_uncore_forcewake_reset(struct drm_device *dev,
bool restore)
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
}
-void intel_uncore_early_sanitize(struct drm_device *dev, bool
restore_forcewake)
+static void __intel_uncore_early_sanitize(struct drm_device *dev,
+ bool restore_forcewake)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -389,6 +390,12 @@ void intel_uncore_early_sanitize(struct drm_device *dev,
bool restore_forcewake)
intel_uncore_forcewake_reset(dev, restore_forcewake);
}
+void intel_uncore_early_sanitize(struct drm_device *dev, bool
restore_forcewake)
+{
+ __intel_uncore_early_sanitize(dev, restore_forcewake);
+ i915_check_and_clear_faults(dev);
+}
+
void intel_uncore_sanitize(struct drm_device *dev)
{
/* BIOS often leaves RC6 enabled, but disable it for hw init */
@@ -833,7 +840,7 @@ void intel_uncore_init(struct drm_device *dev)
setup_timer(&dev_priv->uncore.force_wake_timer,
gen6_force_wake_timer, (unsigned long)dev_priv);
- intel_uncore_early_sanitize(dev, false);
+ __intel_uncore_early_sanitize(dev, false);
if (IS_VALLEYVIEW(dev)) {
dev_priv->uncore.funcs.force_wake_get = __vlv_force_wake_get;
@@ -951,6 +958,8 @@ void intel_uncore_init(struct drm_device *dev)
dev_priv->uncore.funcs.mmio_readq = gen4_read64;
break;
}
+
+ i915_check_and_clear_faults(dev);
}
void intel_uncore_fini(struct drm_device *dev)
--
1.8.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Extend GET_APERTURE ioctl to report available map space
On Mon, Sep 15, 2014 at 04:52:27PM +0300, Konstantin Belousov wrote: > So what will happen when old usermode program (with short old structure) > calls the ioctl ? I believe the memory which happens to be located > after the structure is corrupted, or am I missing some magic there ? > > I.e., the question is why this patch does not break the ABI. The ioctl is buffered in drm_ioctl. Space large enough for the kernel structure is allocated from the heap/stack and the incoming user structure (if required) is copied into the kernel struct and zero extended. After the ioctl, if the struct is an out parameter, what fits into the userspace struct is copied back from the kernel struct. This has the dual benefit of allowing us to extend structures so long as we take care that incoming zeroes from old userspace retain existing behaviour, and vice versa with new userspace and old kernels, and also moves the copy_from_user/copy_to_uesr dance for the majority of cases into a single place (at the cost of giving up some microoptimisations). -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 1/3] drm/i915 Add golden context support for Gen9
Hi Brad,
Thanks for the comments. I'll have to find Damien's SKL patches. I know he's
been on vacation and I haven't looked for them. Mika Kuoppala is submitting
the IGT patches for null_state_gen. They are related to mine, in a sense,
since they generate the intel_renderstate_genx.c files. But I don't think the
IGT and kernel patches have to be submitted together.
Yeah, HEX dumps are pretty cruel. I had to look at them in the windows driver,
but I used our internal tools to disassemble them.
As far as the license header goes, I did add them in manually because the
null_state_gen program simply generates the code. Is there a location in the
git repository for the standard license header?
Thanks,
Armin
-Original Message-
From: Volkin, Bradley D
Sent: Friday, September 12, 2014 5:07 PM
To: Reese, Armin C
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915 Add golden context support for
Gen9
On Fri, Sep 12, 2014 at 11:25:14AM -0700, armin.c.re...@intel.com wrote:
> From: Armin Reese
>
> This patch includes the Gen9 batch buffer to generate a 'golden
> context' for that product family.
>
> Also:
> 1) IS_GEN9 macro has been added to drivers/gpu/drm/i915/i915_drv.h
> 2) drivers/gpu/drm/i915/intel_renderstate_gen8.c has been updated
>to the version created by IGT null_state_gen
Hi Armin,
We'll have to split these extra changes out into separate patches. Damien's SKL
series has the IS_GEN9 macro, which will cause a conflict. For the gen8 change,
better to have a separate patch since it's not strictly related.
I think that the preferred way to handle this situation, at least with respect
to the IS_GEN9 part, is to have a cover letter in which you document the
patches on which your series depends but don't include them in your series.
You'll obviously need to apply those changes locally, just as a separate patch
that you don't send with the series.
Will you be sending the null_state_gen patch that adds the code to generate
this?
I'm not too good at reviewing hex dumps ;)
One other comment below...
>
> Signed-off-by: Armin Reese
> ---
> drivers/gpu/drm/i915/Makefile | 3 +-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +
> drivers/gpu/drm/i915/intel_renderstate.h | 1 +
> drivers/gpu/drm/i915/intel_renderstate_gen6.c | 23 +
> drivers/gpu/drm/i915/intel_renderstate_gen7.c | 23 +
> drivers/gpu/drm/i915/intel_renderstate_gen8.c | 831
> +- drivers/gpu/drm/i915/intel_renderstate_gen9.c
> | 981 ++
> 8 files changed, 1699 insertions(+), 166 deletions(-) create mode
> 100644 drivers/gpu/drm/i915/intel_renderstate_gen9.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile index c1dd485..2caf4f4 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -38,7 +38,8 @@ i915-y += i915_cmd_parser.o \ # autogenerated null
> render state i915-y += intel_renderstate_gen6.o \
> intel_renderstate_gen7.o \
> - intel_renderstate_gen8.o
> + intel_renderstate_gen8.o \
> + intel_renderstate_gen9.o
>
> # modesetting core code
> i915-y += intel_bios.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h index 17dfce0..7d9f091 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2122,6 +2122,7 @@ struct drm_i915_cmd_table {
> #define IS_GEN6(dev) (INTEL_INFO(dev)->gen == 6)
> #define IS_GEN7(dev) (INTEL_INFO(dev)->gen == 7)
> #define IS_GEN8(dev) (INTEL_INFO(dev)->gen == 8)
> +#define IS_GEN9(dev) (INTEL_INFO(dev)->gen == 9)
>
> #define RENDER_RING (1< #define BSD_RING (1< diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c
> b/drivers/gpu/drm/i915/i915_gem_render_state.c
> index a9a62d7..98dcd94 100644
> --- a/drivers/gpu/drm/i915/i915_gem_render_state.c
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -38,6 +38,8 @@ render_state_get_rodata(struct drm_device *dev, const int
> gen)
> return &gen7_null_state;
> case 8:
> return &gen8_null_state;
> + case 9:
> + return &gen9_null_state;
> }
>
> return NULL;
> diff --git a/drivers/gpu/drm/i915/intel_renderstate.h
> b/drivers/gpu/drm/i915/intel_renderstate.h
> index 6c792d3..5bd6985 100644
> --- a/drivers/gpu/drm/i915/intel_renderstate.h
> +++ b/drivers/gpu/drm/i915/intel_renderstate.h
> @@ -29,6 +29,7 @@
> extern const struct intel_renderstate_rodata gen6_null_state; extern
> const struct intel_renderstate_rodata gen7_null_state; extern const
> struct intel_renderstate_rodata gen8_null_state;
> +extern const struct intel_renderstate_rodata gen9_null_state;
>
> #define RO_RENDERSTATE(_g) \
> const struct intel_renderstate_rodata gen ## _g ## _n
Re: [Intel-gfx] [PATCH v2 09/16] drm/i915: check for GT faults in all resume handlers and driver load time
On Mon, Sep 15, 2014 at 06:21:56PM +0300, Imre Deak wrote: > Checking for GT faults is not specific in any way to S4 thaw, so do it > also during S3 resume, S4 restore and driver load time. This allows us to > unify the Sx handlers in an upcoming patch. > > v2: > - move the check to intel_uncore_early_sanitize(), so we check at driver > load time too (Chris) > > Signed-off-by: Imre Deak I stared at early_sanitize and sanitize and decided that what you did here was correct, Reviewed-by: Chris Wilson -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 01/16] drm/i915: vlv: fix gunit HW state corruption during S4 suspend
>From DPM documentation, thaw_early should undo actions by freeze_late.
Can we move following snippet from thaw_early to thaw to comply with
this?
intel_uncore_early_sanitize(dev, true);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
On Wed, 2014-09-10 at 18:16 +0300, Imre Deak wrote:
> During S4 freeze we don't call intel_suspend_complete(), which would
> save the gunit HW state, but during S4 thaw/restore events we call
> intel_resume_prepare() which restores it, thus ending up in a corrupted
> HW state.
>
> Fix this by calling intel_suspend_complete() from the corresponding
> freeze_late event handler.
>
> The issue was introduced in
> commit 016970beb05da6285c2f3ed2bee1c676cb75972e
> Author: Sagar Kamble
> Date: Wed Aug 13 23:07:06 2014 +0530
>
> CC: Sagar Kamble
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/i915/i915_drv.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b8bd008..2365875 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -987,6 +987,15 @@ static int i915_pm_freeze(struct device *dev)
> return i915_drm_freeze(drm_dev);
> }
>
> +static int i915_pm_freeze_late(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct drm_device *drm_dev = pci_get_drvdata(pdev);
> + struct drm_i915_private *dev_priv = drm_dev->dev_private;
> +
> + return intel_suspend_complete(dev_priv);
> +}
> +
> static int i915_pm_thaw_early(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> @@ -1571,6 +1580,7 @@ static const struct dev_pm_ops i915_pm_ops = {
> .resume_early = i915_pm_resume_early,
> .resume = i915_pm_resume,
> .freeze = i915_pm_freeze,
> + .freeze_late = i915_pm_freeze_late,
> .thaw_early = i915_pm_thaw_early,
> .thaw = i915_pm_thaw,
> .poweroff = i915_pm_poweroff,
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/16] drm/i915: vlv: fix gunit HW state corruption during S4 suspend
The point of doing these in thaw_early is to work around an ordering
issue wrt. to the hda driver, see the comment in i915_resume_early(). I
don't see a problem of having them in thaw_early; if you meant that they
lack the corresponding cleanup calls in freeze_late, it's just because
they don't have any.
On Mon, 2014-09-15 at 22:56 +0530, Sagar Arun Kamble wrote:
> From DPM documentation, thaw_early should undo actions by freeze_late.
> Can we move following snippet from thaw_early to thaw to comply with
> this?
>
> intel_uncore_early_sanitize(dev, true);
> intel_uncore_sanitize(dev);
> intel_power_domains_init_hw(dev_priv);
>
> On Wed, 2014-09-10 at 18:16 +0300, Imre Deak wrote:
> > During S4 freeze we don't call intel_suspend_complete(), which would
> > save the gunit HW state, but during S4 thaw/restore events we call
> > intel_resume_prepare() which restores it, thus ending up in a corrupted
> > HW state.
> >
> > Fix this by calling intel_suspend_complete() from the corresponding
> > freeze_late event handler.
> >
> > The issue was introduced in
> > commit 016970beb05da6285c2f3ed2bee1c676cb75972e
> > Author: Sagar Kamble
> > Date: Wed Aug 13 23:07:06 2014 +0530
> >
> > CC: Sagar Kamble
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/i915/i915_drv.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index b8bd008..2365875 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -987,6 +987,15 @@ static int i915_pm_freeze(struct device *dev)
> > return i915_drm_freeze(drm_dev);
> > }
> >
> > +static int i915_pm_freeze_late(struct device *dev)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > + struct drm_i915_private *dev_priv = drm_dev->dev_private;
> > +
> > + return intel_suspend_complete(dev_priv);
> > +}
> > +
> > static int i915_pm_thaw_early(struct device *dev)
> > {
> > struct pci_dev *pdev = to_pci_dev(dev);
> > @@ -1571,6 +1580,7 @@ static const struct dev_pm_ops i915_pm_ops = {
> > .resume_early = i915_pm_resume_early,
> > .resume = i915_pm_resume,
> > .freeze = i915_pm_freeze,
> > + .freeze_late = i915_pm_freeze_late,
> > .thaw_early = i915_pm_thaw_early,
> > .thaw = i915_pm_thaw,
> > .poweroff = i915_pm_poweroff,
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Fix Sink CRC
In some cases like when PSR just got enabled the panel need more vblank
times to calculate CRC. I figured that out with the new PSR test cases
facing some cases that I had a green screen but a blank CRC. Even with
2 vblank waits on kernel + 2 vblank waits on test case.
So let's give up to 6 vblank wait time. However we now check for
TEST_CRC_COUNT that shows when panel finished to calculate CRC and
has it ready.
v2: Jani pointed out attempts decrements was wrong and should never reach
the error condition. And Daniel pointed out that EIO is more appropriated than
EGAIN. Also I realized that I have to read test_crc_count after setting
test_sink
Cc: Daniel Vetter
Cc: Jani Nikula
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_dp.c | 21 +++--
include/drm/drm_dp_helper.h | 5 +++--
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f79473b..fae0fae 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3468,21 +3468,30 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8
*crc)
struct drm_device *dev = intel_dig_port->base.base.dev;
struct intel_crtc *intel_crtc =
to_intel_crtc(intel_dig_port->base.base.crtc);
- u8 buf[1];
+ u8 buf;
+ int test_crc_count;
+ int attempts = 6;
- if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, buf) < 0)
+ if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
return -EAGAIN;
- if (!(buf[0] & DP_TEST_CRC_SUPPORTED))
+ if (!(buf & DP_TEST_CRC_SUPPORTED))
return -ENOTTY;
if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK,
DP_TEST_SINK_START) < 0)
return -EAGAIN;
- /* Wait 2 vblanks to be sure we will have the correct CRC value */
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- intel_wait_for_vblank(dev, intel_crtc->pipe);
+ drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
+ test_crc_count = buf & DP_TEST_COUNT_MASK;
+
+ do {
+ drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf);
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ } while (--attempts && (buf & DP_TEST_COUNT_MASK) == test_crc_count);
+
+ if (attempts == 0)
+ return -EIO;
if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
return -EAGAIN;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 9305c71..8edeed0 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -303,7 +303,8 @@
#define DP_TEST_CRC_B_CB 0x244
#define DP_TEST_SINK_MISC 0x246
-#define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_CRC_SUPPORTED (1 << 5)
+# define DP_TEST_COUNT_MASK0x7
#define DP_TEST_RESPONSE 0x260
# define DP_TEST_ACK (1 << 0)
@@ -313,7 +314,7 @@
#define DP_TEST_EDID_CHECKSUM 0x261
#define DP_TEST_SINK 0x270
-#define DP_TEST_SINK_START (1 << 0)
+# define DP_TEST_SINK_START(1 << 0)
#define DP_PAYLOAD_TABLE_UPDATE_STATUS 0x2c0 /* 1.2 MST */
# define DP_PAYLOAD_TABLE_UPDATED (1 << 0)
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Use EIO instead of EAGAIN for sink CRC error.
If something while getting panel CRC this means that probably hw I/O error so hw is busted and try again shouldn't help much. Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/i915/intel_dp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index fae0fae..963e956 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3473,14 +3473,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) int attempts = 6; if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0) - return -EAGAIN; + return -EIO; if (!(buf & DP_TEST_CRC_SUPPORTED)) return -ENOTTY; if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, DP_TEST_SINK_START) < 0) - return -EAGAIN; + return -EIO; drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf); test_crc_count = buf & DP_TEST_COUNT_MASK; @@ -3494,7 +3494,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) return -EIO; if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) - return -EAGAIN; + return -EIO; drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, 0); return 0; -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
