* [PATCH 01/10] drm/i915: Parametrize cursor/primary pipe select bits
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 21:51 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 02/10] drm/i915: Pass intel_plane and intel_crtc to plane hooks ville.syrjala
` (9 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_reg.h | 7 ++-----
drivers/gpu/drm/i915/intel_display.c | 6 +++---
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cc843f96576f..07cae03e8727 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5439,9 +5439,7 @@ enum {
#define CURSOR_MODE_128_ARGB_AX ((1 << 5) | CURSOR_MODE_128_32B_AX)
#define CURSOR_MODE_256_ARGB_AX ((1 << 5) | CURSOR_MODE_256_32B_AX)
#define CURSOR_MODE_64_ARGB_AX ((1 << 5) | CURSOR_MODE_64_32B_AX)
-#define MCURSOR_PIPE_SELECT (1 << 28)
-#define MCURSOR_PIPE_A 0x00
-#define MCURSOR_PIPE_B (1 << 28)
+#define MCURSOR_PIPE_SELECT(pipe) ((pipe) << 28)
#define MCURSOR_GAMMA_ENABLE (1 << 26)
#define CURSOR_ROTATE_180 (1<<15)
#define CURSOR_TRICKLE_FEED_DISABLE (1 << 14)
@@ -5499,8 +5497,7 @@ enum {
#define DISPPLANE_PIPE_CSC_ENABLE (1<<24)
#define DISPPLANE_SEL_PIPE_SHIFT 24
#define DISPPLANE_SEL_PIPE_MASK (3<<DISPPLANE_SEL_PIPE_SHIFT)
-#define DISPPLANE_SEL_PIPE_A 0
-#define DISPPLANE_SEL_PIPE_B (1<<DISPPLANE_SEL_PIPE_SHIFT)
+#define DISPPLANE_SEL_PIPE(pipe) ((pipe)<<DISPPLANE_SEL_PIPE_SHIFT)
#define DISPPLANE_SRC_KEY_ENABLE (1<<22)
#define DISPPLANE_SRC_KEY_DISABLE 0
#define DISPPLANE_LINE_DOUBLE (1<<20)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77ca7dfa44d..4e8351d89d10 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2989,8 +2989,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
dspcntr |= DISPLAY_PLANE_ENABLE;
if (INTEL_GEN(dev_priv) < 4) {
- if (intel_crtc->pipe == PIPE_B)
- dspcntr |= DISPPLANE_SEL_PIPE_B;
+ dspcntr |= DISPPLANE_SEL_PIPE(intel_crtc->pipe);
/* pipesrc and dspsize control the size that is scaled from,
* which should always be the user's requested size.
@@ -9247,7 +9246,8 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
MISSING_CASE(plane_state->base.crtc_w);
return;
}
- cntl |= pipe << 28; /* Connect to correct pipe */
+
+ cntl |= MCURSOR_PIPE_SELECT(pipe);
if (HAS_DDI(dev_priv))
cntl |= CURSOR_PIPE_CSC_ENABLE;
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 02/10] drm/i915: Pass intel_plane and intel_crtc to plane hooks
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
2017-03-07 15:27 ` [PATCH 01/10] drm/i915: Parametrize cursor/primary pipe select bits ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 21:50 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 03/10] drm/i915: Refactor CURBASE calculation ville.syrjala
` (8 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Streamline things by passing intel_plane and intel_crtc instead of
the drm types to our plane hooks.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_atomic_plane.c | 6 +-
drivers/gpu/drm/i915/intel_display.c | 123 ++++++++++++++----------------
drivers/gpu/drm/i915/intel_drv.h | 8 +-
drivers/gpu/drm/i915/intel_sprite.c | 102 ++++++++++---------------
4 files changed, 107 insertions(+), 132 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index cfb47293fd53..182dc2a36ed1 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -185,7 +185,7 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
}
intel_state->base.visible = false;
- ret = intel_plane->check_plane(plane, crtc_state, intel_state);
+ ret = intel_plane->check_plane(intel_plane, crtc_state, intel_state);
if (ret)
return ret;
@@ -235,14 +235,14 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
trace_intel_update_plane(plane,
to_intel_crtc(crtc));
- intel_plane->update_plane(plane,
+ intel_plane->update_plane(intel_plane,
to_intel_crtc_state(crtc->state),
intel_state);
} else {
trace_intel_disable_plane(plane,
to_intel_crtc(crtc));
- intel_plane->disable_plane(plane, crtc);
+ intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
}
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4e8351d89d10..c3e064af3b43 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2755,7 +2755,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
false);
intel_pre_disable_primary_noatomic(&intel_crtc->base);
trace_intel_disable_plane(primary, intel_crtc);
- intel_plane->disable_plane(primary, &intel_crtc->base);
+ intel_plane->disable_plane(intel_plane, intel_crtc);
return;
@@ -2969,14 +2969,14 @@ int skl_check_plane_surface(struct intel_plane_state *plane_state)
return 0;
}
-static void i9xx_update_primary_plane(struct drm_plane *primary,
+static void i9xx_update_primary_plane(struct intel_plane *primary,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_i915_private *dev_priv = to_i915(primary->dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
- int plane = intel_crtc->plane;
+ int plane = crtc->plane;
u32 linear_offset;
u32 dspcntr;
i915_reg_t reg = DSPCNTR(plane);
@@ -2989,7 +2989,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
dspcntr |= DISPLAY_PLANE_ENABLE;
if (INTEL_GEN(dev_priv) < 4) {
- dspcntr |= DISPPLANE_SEL_PIPE(intel_crtc->pipe);
+ dspcntr |= DISPPLANE_SEL_PIPE(crtc->pipe);
/* pipesrc and dspsize control the size that is scaled from,
* which should always be the user's requested size.
@@ -3048,7 +3048,7 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
intel_add_fb_offsets(&x, &y, plane_state, 0);
if (INTEL_GEN(dev_priv) >= 4)
- intel_crtc->dspaddr_offset =
+ crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, plane_state, 0);
if (rotation & DRM_ROTATE_180) {
@@ -3061,10 +3061,10 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
if (INTEL_GEN(dev_priv) < 4)
- intel_crtc->dspaddr_offset = linear_offset;
+ crtc->dspaddr_offset = linear_offset;
- intel_crtc->adjusted_x = x;
- intel_crtc->adjusted_y = y;
+ crtc->adjusted_x = x;
+ crtc->adjusted_y = y;
I915_WRITE(reg, dspcntr);
@@ -3072,24 +3072,22 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
if (INTEL_GEN(dev_priv) >= 4) {
I915_WRITE(DSPSURF(plane),
intel_plane_ggtt_offset(plane_state) +
- intel_crtc->dspaddr_offset);
+ crtc->dspaddr_offset);
I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
I915_WRITE(DSPLINOFF(plane), linear_offset);
} else {
I915_WRITE(DSPADDR(plane),
intel_plane_ggtt_offset(plane_state) +
- intel_crtc->dspaddr_offset);
+ crtc->dspaddr_offset);
}
POSTING_READ(reg);
}
-static void i9xx_disable_primary_plane(struct drm_plane *primary,
- struct drm_crtc *crtc)
+static void i9xx_disable_primary_plane(struct intel_plane *primary,
+ struct intel_crtc *crtc)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int plane = intel_crtc->plane;
+ struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+ int plane = crtc->plane;
I915_WRITE(DSPCNTR(plane), 0);
if (INTEL_INFO(dev_priv)->gen >= 4)
@@ -3099,15 +3097,14 @@ static void i9xx_disable_primary_plane(struct drm_plane *primary,
POSTING_READ(DSPCNTR(plane));
}
-static void ironlake_update_primary_plane(struct drm_plane *primary,
+static void ironlake_update_primary_plane(struct intel_plane *primary,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = primary->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
- int plane = intel_crtc->plane;
+ int plane = crtc->plane;
u32 linear_offset;
u32 dspcntr;
i915_reg_t reg = DSPCNTR(plane);
@@ -3155,7 +3152,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
intel_add_fb_offsets(&x, &y, plane_state, 0);
- intel_crtc->dspaddr_offset =
+ crtc->dspaddr_offset =
intel_compute_tile_offset(&x, &y, plane_state, 0);
/* HSW+ does this automagically in hardware */
@@ -3167,15 +3164,15 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
- intel_crtc->adjusted_x = x;
- intel_crtc->adjusted_y = y;
+ crtc->adjusted_x = x;
+ crtc->adjusted_y = y;
I915_WRITE(reg, dspcntr);
I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
I915_WRITE(DSPSURF(plane),
intel_plane_ggtt_offset(plane_state) +
- intel_crtc->dspaddr_offset);
+ crtc->dspaddr_offset);
if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
} else {
@@ -3327,16 +3324,15 @@ u32 skl_plane_ctl_rotation(unsigned int rotation)
return 0;
}
-static void skylake_update_primary_plane(struct drm_plane *plane,
+static void skylake_update_primary_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = plane_state->base.fb;
- enum plane_id plane_id = to_intel_plane(plane)->id;
- enum pipe pipe = to_intel_plane(plane)->pipe;
+ enum plane_id plane_id = plane->id;
+ enum pipe pipe = plane->pipe;
u32 plane_ctl;
unsigned int rotation = plane_state->base.rotation;
u32 stride = skl_plane_stride(fb, 0, rotation);
@@ -3375,10 +3371,10 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
dst_w--;
dst_h--;
- intel_crtc->dspaddr_offset = surf_addr;
+ crtc->dspaddr_offset = surf_addr;
- intel_crtc->adjusted_x = src_x;
- intel_crtc->adjusted_y = src_y;
+ crtc->adjusted_x = src_x;
+ crtc->adjusted_y = src_y;
I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
I915_WRITE(PLANE_OFFSET(pipe, plane_id), (src_y << 16) | src_x);
@@ -3406,13 +3402,12 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
POSTING_READ(PLANE_SURF(pipe, plane_id));
}
-static void skylake_disable_primary_plane(struct drm_plane *primary,
- struct drm_crtc *crtc)
+static void skylake_disable_primary_plane(struct intel_plane *primary,
+ struct intel_crtc *crtc)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- enum plane_id plane_id = to_intel_plane(primary)->id;
- enum pipe pipe = to_intel_plane(primary)->pipe;
+ struct drm_i915_private *dev_priv = to_i915(primary->base.dev);
+ enum plane_id plane_id = primary->id;
+ enum pipe pipe = primary->pipe;
I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
I915_WRITE(PLANE_SURF(pipe, plane_id), 0);
@@ -3451,7 +3446,7 @@ static void intel_update_primary_planes(struct drm_device *dev)
trace_intel_update_plane(&plane->base,
to_intel_crtc(crtc));
- plane->update_plane(&plane->base,
+ plane->update_plane(plane,
to_intel_crtc_state(crtc->state),
plane_state);
}
@@ -5110,7 +5105,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask
intel_crtc_dpms_overlay_disable(intel_crtc);
drm_for_each_plane_mask(p, dev, plane_mask)
- to_intel_plane(p)->disable_plane(p, crtc);
+ to_intel_plane(p)->disable_plane(to_intel_plane(p), intel_crtc);
/*
* FIXME: Once we grow proper nuclear flip support out of this we need
@@ -13291,11 +13286,11 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
}
static int
-intel_check_primary_plane(struct drm_plane *plane,
+intel_check_primary_plane(struct intel_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state)
{
- struct drm_i915_private *dev_priv = to_i915(plane->dev);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_crtc *crtc = state->base.crtc;
int min_scale = DRM_PLANE_HELPER_NO_SCALING;
int max_scale = DRM_PLANE_HELPER_NO_SCALING;
@@ -13501,12 +13496,12 @@ intel_legacy_cursor_update(struct drm_plane *plane,
if (plane->state->visible) {
trace_intel_update_plane(plane, to_intel_crtc(crtc));
- intel_plane->update_plane(plane,
+ intel_plane->update_plane(intel_plane,
to_intel_crtc_state(crtc->state),
to_intel_plane_state(plane->state));
} else {
trace_intel_disable_plane(plane, to_intel_crtc(crtc));
- intel_plane->disable_plane(plane, crtc);
+ intel_plane->disable_plane(intel_plane, to_intel_crtc(crtc));
}
intel_cleanup_plane_fb(plane, new_plane_state);
@@ -13656,13 +13651,14 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
}
static int
-intel_check_cursor_plane(struct drm_plane *plane,
+intel_check_cursor_plane(struct intel_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state)
{
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- enum pipe pipe = to_intel_plane(plane)->pipe;
+ enum pipe pipe = plane->pipe;
unsigned stride;
int ret;
@@ -13679,7 +13675,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
return 0;
/* Check for which cursor types we support */
- if (!cursor_size_ok(to_i915(plane->dev), state->base.crtc_w,
+ if (!cursor_size_ok(dev_priv, state->base.crtc_w,
state->base.crtc_h)) {
DRM_DEBUG("Cursor dimension %dx%d not supported\n",
state->base.crtc_w, state->base.crtc_h);
@@ -13707,7 +13703,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
* display power well must be turned off and on again.
* Refuse the put the cursor into that compromised position.
*/
- if (IS_CHERRYVIEW(to_i915(plane->dev)) && pipe == PIPE_C &&
+ if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
state->base.visible && state->base.crtc_x < 0) {
DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
return -EINVAL;
@@ -13717,23 +13713,20 @@ intel_check_cursor_plane(struct drm_plane *plane,
}
static void
-intel_disable_cursor_plane(struct drm_plane *plane,
- struct drm_crtc *crtc)
+intel_disable_cursor_plane(struct intel_plane *plane,
+ struct intel_crtc *crtc)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- intel_crtc->cursor_addr = 0;
- intel_crtc_update_cursor(crtc, NULL);
+ crtc->cursor_addr = 0;
+ intel_crtc_update_cursor(&crtc->base, NULL);
}
static void
-intel_update_cursor_plane(struct drm_plane *plane,
+intel_update_cursor_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *state)
{
- struct drm_crtc *crtc = crtc_state->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct drm_i915_private *dev_priv = to_i915(plane->dev);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
uint32_t addr;
@@ -13744,8 +13737,8 @@ intel_update_cursor_plane(struct drm_plane *plane,
else
addr = obj->phys_handle->busaddr;
- intel_crtc->cursor_addr = addr;
- intel_crtc_update_cursor(crtc, state);
+ crtc->cursor_addr = addr;
+ intel_crtc_update_cursor(&crtc->base, state);
}
static struct intel_plane *
@@ -15157,7 +15150,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
continue;
trace_intel_disable_plane(&plane->base, crtc);
- plane->disable_plane(&plane->base, &crtc->base);
+ plane->disable_plane(plane, crtc);
}
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0f766f83a31b..54382f1f961c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -808,12 +808,12 @@ struct intel_plane {
* the intel_plane_state structure and accessed via plane_state.
*/
- void (*update_plane)(struct drm_plane *plane,
+ void (*update_plane)(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state);
- void (*disable_plane)(struct drm_plane *plane,
- struct drm_crtc *crtc);
- int (*check_plane)(struct drm_plane *plane,
+ void (*disable_plane)(struct intel_plane *plane,
+ struct intel_crtc *crtc);
+ int (*check_plane)(struct intel_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state);
};
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 375ca91b308c..f10cb9b5c37e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,16 +203,14 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
}
static void
-skl_update_plane(struct drm_plane *drm_plane,
+skl_update_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = drm_plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(drm_plane);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = plane_state->base.fb;
- enum plane_id plane_id = intel_plane->id;
- enum pipe pipe = intel_plane->pipe;
+ enum plane_id plane_id = plane->id;
+ enum pipe pipe = plane->pipe;
u32 plane_ctl;
const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
u32 surf_addr = plane_state->main.offset;
@@ -295,13 +293,11 @@ skl_update_plane(struct drm_plane *drm_plane,
}
static void
-skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
+skl_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
{
- struct drm_device *dev = dplane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(dplane);
- enum plane_id plane_id = intel_plane->id;
- enum pipe pipe = intel_plane->pipe;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum plane_id plane_id = plane->id;
+ enum pipe pipe = plane->pipe;
I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
@@ -310,10 +306,10 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
}
static void
-chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
+chv_update_csc(struct intel_plane *plane, uint32_t format)
{
- struct drm_i915_private *dev_priv = to_i915(intel_plane->base.dev);
- enum plane_id plane_id = intel_plane->id;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum plane_id plane_id = plane->id;
/* Seems RGB data bypasses the CSC always */
if (!format_is_yuv(format))
@@ -349,16 +345,14 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
}
static void
-vlv_update_plane(struct drm_plane *dplane,
+vlv_update_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = dplane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(dplane);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = plane_state->base.fb;
- enum pipe pipe = intel_plane->pipe;
- enum plane_id plane_id = intel_plane->id;
+ enum pipe pipe = plane->pipe;
+ enum plane_id plane_id = plane->id;
u32 sprctl;
u32 sprsurf_offset, linear_offset;
unsigned int rotation = plane_state->base.rotation;
@@ -460,7 +454,7 @@ vlv_update_plane(struct drm_plane *dplane,
sprctl |= SP_SOURCE_KEY;
if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_B)
- chv_update_csc(intel_plane, fb->format->format);
+ chv_update_csc(plane, fb->format->format);
I915_WRITE(SPSTRIDE(pipe, plane_id), fb->pitches[0]);
I915_WRITE(SPPOS(pipe, plane_id), (crtc_y << 16) | crtc_x);
@@ -480,13 +474,11 @@ vlv_update_plane(struct drm_plane *dplane,
}
static void
-vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
+vlv_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
{
- struct drm_device *dev = dplane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(dplane);
- enum pipe pipe = intel_plane->pipe;
- enum plane_id plane_id = intel_plane->id;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum pipe pipe = plane->pipe;
+ enum plane_id plane_id = plane->id;
I915_WRITE(SPCNTR(pipe, plane_id), 0);
@@ -495,15 +487,13 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
}
static void
-ivb_update_plane(struct drm_plane *plane,
+ivb_update_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = plane_state->base.fb;
- enum pipe pipe = intel_plane->pipe;
+ enum pipe pipe = plane->pipe;
u32 sprctl, sprscale = 0;
u32 sprsurf_offset, linear_offset;
unsigned int rotation = plane_state->base.rotation;
@@ -607,7 +597,7 @@ ivb_update_plane(struct drm_plane *plane,
I915_WRITE(SPRLINOFF(pipe), linear_offset);
I915_WRITE(SPRSIZE(pipe), (crtc_h << 16) | crtc_w);
- if (intel_plane->can_scale)
+ if (plane->can_scale)
I915_WRITE(SPRSCALE(pipe), sprscale);
I915_WRITE(SPRCTL(pipe), sprctl);
I915_WRITE(SPRSURF(pipe),
@@ -616,16 +606,14 @@ ivb_update_plane(struct drm_plane *plane,
}
static void
-ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ivb_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(plane);
- int pipe = intel_plane->pipe;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum pipe pipe = plane->pipe;
I915_WRITE(SPRCTL(pipe), 0);
/* Can't leave the scaler enabled... */
- if (intel_plane->can_scale)
+ if (plane->can_scale)
I915_WRITE(SPRSCALE(pipe), 0);
I915_WRITE(SPRSURF(pipe), 0);
@@ -633,15 +621,13 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
}
static void
-ilk_update_plane(struct drm_plane *plane,
+ilk_update_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = plane_state->base.fb;
- int pipe = intel_plane->pipe;
+ enum pipe pipe = plane->pipe;
u32 dvscntr, dvsscale;
u32 dvssurf_offset, linear_offset;
unsigned int rotation = plane_state->base.rotation;
@@ -743,12 +729,10 @@ ilk_update_plane(struct drm_plane *plane,
}
static void
-ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ilk_disable_plane(struct intel_plane *plane, struct intel_crtc *crtc)
{
- struct drm_device *dev = plane->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_plane *intel_plane = to_intel_plane(plane);
- int pipe = intel_plane->pipe;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum pipe pipe = plane->pipe;
I915_WRITE(DVSCNTR(pipe), 0);
/* Disable the scaler */
@@ -759,14 +743,12 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
}
static int
-intel_check_sprite_plane(struct drm_plane *plane,
+intel_check_sprite_plane(struct intel_plane *plane,
struct intel_crtc_state *crtc_state,
struct intel_plane_state *state)
{
- struct drm_i915_private *dev_priv = to_i915(plane->dev);
- struct drm_crtc *crtc = state->base.crtc;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_framebuffer *fb = state->base.fb;
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
@@ -788,7 +770,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
}
/* Don't modify another pipe's plane */
- if (intel_plane->pipe != intel_crtc->pipe) {
+ if (plane->pipe != crtc->pipe) {
DRM_DEBUG_KMS("Wrong plane <-> crtc mapping\n");
return -EINVAL;
}
@@ -805,16 +787,16 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (state->ckey.flags == I915_SET_COLORKEY_NONE) {
can_scale = 1;
min_scale = 1;
- max_scale = skl_max_scale(intel_crtc, crtc_state);
+ max_scale = skl_max_scale(crtc, crtc_state);
} else {
can_scale = 0;
min_scale = DRM_PLANE_HELPER_NO_SCALING;
max_scale = DRM_PLANE_HELPER_NO_SCALING;
}
} else {
- can_scale = intel_plane->can_scale;
- max_scale = intel_plane->max_downscale << 16;
- min_scale = intel_plane->can_scale ? 1 : (1 << 16);
+ can_scale = plane->can_scale;
+ max_scale = plane->max_downscale << 16;
+ min_scale = plane->can_scale ? 1 : (1 << 16);
}
/*
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 03/10] drm/i915: Refactor CURBASE calculation
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
2017-03-07 15:27 ` [PATCH 01/10] drm/i915: Parametrize cursor/primary pipe select bits ville.syrjala
2017-03-07 15:27 ` [PATCH 02/10] drm/i915: Pass intel_plane and intel_crtc to plane hooks ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 21:56 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc ville.syrjala
` (7 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The remaining cursor base address calculations are spread
around into several different locations. Just pull it all
into one place.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 54 +++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c3e064af3b43..b884c0d5bbb3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9153,6 +9153,31 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
return active;
}
+static u32 intel_cursor_base(struct intel_crtc *crtc,
+ struct intel_plane *plane,
+ const struct intel_plane_state *plane_state)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ const struct drm_framebuffer *fb = plane_state->base.fb;
+ const struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ u32 base;
+
+ if (INTEL_INFO(dev_priv)->cursor_needs_physical)
+ base = obj->phys_handle->busaddr;
+ else
+ base = intel_plane_ggtt_offset(plane_state);
+
+ crtc->cursor_addr = base;
+
+ /* ILK+ do this automagically */
+ if (HAS_GMCH_DISPLAY(dev_priv) &&
+ plane_state->base.rotation & DRM_ROTATE_180)
+ base += (plane_state->base.crtc_h *
+ plane_state->base.crtc_w - 1) * fb->format->cpp[0];
+
+ return base;
+}
+
static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
const struct intel_plane_state *plane_state)
{
@@ -9266,14 +9291,14 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
static void intel_crtc_update_cursor(struct drm_crtc *crtc,
+ struct intel_plane *plane,
const struct intel_plane_state *plane_state)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc->pipe;
- u32 base = intel_crtc->cursor_addr;
- u32 pos = 0;
+ u32 pos = 0, base = 0;
if (plane_state) {
int x = plane_state->base.crtc_x;
@@ -9291,12 +9316,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
}
pos |= y << CURSOR_Y_SHIFT;
- /* ILK+ do this automagically */
- if (HAS_GMCH_DISPLAY(dev_priv) &&
- plane_state->base.rotation & DRM_ROTATE_180) {
- base += (plane_state->base.crtc_h *
- plane_state->base.crtc_w - 1) * 4;
- }
+ base = intel_cursor_base(intel_crtc, plane, plane_state);
+ } else {
+ intel_crtc->cursor_addr = 0;
}
I915_WRITE(CURPOS(pipe), pos);
@@ -13716,8 +13738,7 @@ static void
intel_disable_cursor_plane(struct intel_plane *plane,
struct intel_crtc *crtc)
{
- crtc->cursor_addr = 0;
- intel_crtc_update_cursor(&crtc->base, NULL);
+ intel_crtc_update_cursor(&crtc->base, plane, NULL);
}
static void
@@ -13725,20 +13746,9 @@ intel_update_cursor_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *state)
{
- struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
- struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
- uint32_t addr;
-
- if (!obj)
- addr = 0;
- else if (!INTEL_INFO(dev_priv)->cursor_needs_physical)
- addr = intel_plane_ggtt_offset(state);
- else
- addr = obj->phys_handle->busaddr;
- crtc->cursor_addr = addr;
- intel_crtc_update_cursor(&crtc->base, state);
+ intel_crtc_update_cursor(&crtc->base, plane, state);
}
static struct intel_plane *
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (2 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 03/10] drm/i915: Refactor CURBASE calculation ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:33 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 05/10] drm/i915: Refactor CURPOS calculation ville.syrjala
` (6 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Move cursor_base, cursor_cntl, and cursor_size from intel_crtc
into intel_plane so that we don't need the crtc for cursor stuff
so much.
Also entirely nuke cursor_addr which IMO doesn't provide any benefit
since it's not actually used by the cursor code itself. I'm not 100%
sure what the SKL+ DDB is code is after by looking at cursor_addr so
I just make it do its checks unconditionally. If that's not correct
then we should likely replace it with somehting like
plane_state->visible.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 48 +++++-----------------
drivers/gpu/drm/i915/intel_display.c | 80 +++++++++++++++---------------------
drivers/gpu/drm/i915/intel_drv.h | 9 ++--
3 files changed, 47 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index aa2d726b4349..fc20580ecfc3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3031,36 +3031,6 @@ static void intel_connector_info(struct seq_file *m,
intel_seq_print_mode(m, 2, mode);
}
-static bool cursor_active(struct drm_i915_private *dev_priv, int pipe)
-{
- u32 state;
-
- if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
- state = I915_READ(CURCNTR(PIPE_A)) & CURSOR_ENABLE;
- else
- state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
-
- return state;
-}
-
-static bool cursor_position(struct drm_i915_private *dev_priv,
- int pipe, int *x, int *y)
-{
- u32 pos;
-
- pos = I915_READ(CURPOS(pipe));
-
- *x = (pos >> CURSOR_X_SHIFT) & CURSOR_POS_MASK;
- if (pos & (CURSOR_POS_SIGN << CURSOR_X_SHIFT))
- *x = -*x;
-
- *y = (pos >> CURSOR_Y_SHIFT) & CURSOR_POS_MASK;
- if (pos & (CURSOR_POS_SIGN << CURSOR_Y_SHIFT))
- *y = -*y;
-
- return cursor_active(dev_priv, pipe);
-}
-
static const char *plane_type(enum drm_plane_type type)
{
switch (type) {
@@ -3182,9 +3152,7 @@ static int i915_display_info(struct seq_file *m, void *unused)
seq_printf(m, "CRTC info\n");
seq_printf(m, "---------\n");
for_each_intel_crtc(dev, crtc) {
- bool active;
struct intel_crtc_state *pipe_config;
- int x, y;
pipe_config = to_intel_crtc_state(crtc->base.state);
@@ -3195,14 +3163,18 @@ static int i915_display_info(struct seq_file *m, void *unused)
yesno(pipe_config->dither), pipe_config->pipe_bpp);
if (pipe_config->base.active) {
+ struct intel_plane *cursor =
+ to_intel_plane(crtc->base.cursor);
+
intel_crtc_info(m, crtc);
- active = cursor_position(dev_priv, crtc->pipe, &x, &y);
- seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x, active? %s\n",
- yesno(crtc->cursor_base),
- x, y, crtc->base.cursor->state->crtc_w,
- crtc->base.cursor->state->crtc_h,
- crtc->cursor_addr, yesno(active));
+ seq_printf(m, "\tcursor visible? %s, position (%d, %d), size %dx%d, addr 0x%08x\n",
+ yesno(cursor->base.state->visible),
+ cursor->base.state->crtc_x,
+ cursor->base.state->crtc_y,
+ cursor->base.state->crtc_w,
+ cursor->base.state->crtc_h,
+ cursor->cursor.base);
intel_scaler_info(m, crtc);
intel_plane_info(m, crtc);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b884c0d5bbb3..191685a37200 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9153,8 +9153,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
return active;
}
-static u32 intel_cursor_base(struct intel_crtc *crtc,
- struct intel_plane *plane,
+static u32 intel_cursor_base(struct intel_plane *plane,
const struct intel_plane_state *plane_state)
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
@@ -9167,8 +9166,6 @@ static u32 intel_cursor_base(struct intel_crtc *crtc,
else
base = intel_plane_ggtt_offset(plane_state);
- crtc->cursor_addr = base;
-
/* ILK+ do this automagically */
if (HAS_GMCH_DISPLAY(dev_priv) &&
plane_state->base.rotation & DRM_ROTATE_180)
@@ -9178,12 +9175,10 @@ static u32 intel_cursor_base(struct intel_crtc *crtc,
return base;
}
-static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
+static void i845_update_cursor(struct intel_plane *plane, u32 base,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
uint32_t cntl = 0, size = 0;
if (plane_state && plane_state->base.visible) {
@@ -9212,42 +9207,40 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
size = (height << 12) | width;
}
- if (intel_crtc->cursor_cntl != 0 &&
- (intel_crtc->cursor_base != base ||
- intel_crtc->cursor_size != size ||
- intel_crtc->cursor_cntl != cntl)) {
+ if (plane->cursor.cntl != 0 &&
+ (plane->cursor.base != base ||
+ plane->cursor.size != size ||
+ plane->cursor.cntl != cntl)) {
/* On these chipsets we can only modify the base/size/stride
* whilst the cursor is disabled.
*/
I915_WRITE(CURCNTR(PIPE_A), 0);
POSTING_READ(CURCNTR(PIPE_A));
- intel_crtc->cursor_cntl = 0;
+ plane->cursor.cntl = 0;
}
- if (intel_crtc->cursor_base != base) {
+ if (plane->cursor.base != base) {
I915_WRITE(CURBASE(PIPE_A), base);
- intel_crtc->cursor_base = base;
+ plane->cursor.base = base;
}
- if (intel_crtc->cursor_size != size) {
+ if (plane->cursor.size != size) {
I915_WRITE(CURSIZE, size);
- intel_crtc->cursor_size = size;
+ plane->cursor.size = size;
}
- if (intel_crtc->cursor_cntl != cntl) {
+ if (plane->cursor.cntl != cntl) {
I915_WRITE(CURCNTR(PIPE_A), cntl);
POSTING_READ(CURCNTR(PIPE_A));
- intel_crtc->cursor_cntl = cntl;
+ plane->cursor.cntl = cntl;
}
}
-static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
+static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum pipe pipe = plane->pipe;
uint32_t cntl = 0;
if (plane_state && plane_state->base.visible) {
@@ -9276,28 +9269,25 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
cntl |= CURSOR_ROTATE_180;
}
- if (intel_crtc->cursor_cntl != cntl) {
+ if (plane->cursor.cntl != cntl) {
I915_WRITE(CURCNTR(pipe), cntl);
POSTING_READ(CURCNTR(pipe));
- intel_crtc->cursor_cntl = cntl;
+ plane->cursor.cntl = cntl;
}
/* and commit changes on next vblank */
I915_WRITE(CURBASE(pipe), base);
POSTING_READ(CURBASE(pipe));
- intel_crtc->cursor_base = base;
+ plane->cursor.base = base;
}
/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
-static void intel_crtc_update_cursor(struct drm_crtc *crtc,
- struct intel_plane *plane,
+static void intel_crtc_update_cursor(struct intel_plane *plane,
const struct intel_plane_state *plane_state)
{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc->pipe;
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ enum pipe pipe = plane->pipe;
u32 pos = 0, base = 0;
if (plane_state) {
@@ -9316,17 +9306,15 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
}
pos |= y << CURSOR_Y_SHIFT;
- base = intel_cursor_base(intel_crtc, plane, plane_state);
- } else {
- intel_crtc->cursor_addr = 0;
+ base = intel_cursor_base(plane, plane_state);
}
I915_WRITE(CURPOS(pipe), pos);
if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
- i845_update_cursor(crtc, base, plane_state);
+ i845_update_cursor(plane, base, plane_state);
else
- i9xx_update_cursor(crtc, base, plane_state);
+ i9xx_update_cursor(plane, base, plane_state);
}
static bool cursor_size_ok(struct drm_i915_private *dev_priv,
@@ -11882,7 +11870,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
* allocation. In that case since the ddb allocation will be updated
* once the plane becomes visible, we can skip this check
*/
- if (intel_crtc->cursor_addr) {
+ if (1) {
hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
@@ -13738,7 +13726,7 @@ static void
intel_disable_cursor_plane(struct intel_plane *plane,
struct intel_crtc *crtc)
{
- intel_crtc_update_cursor(&crtc->base, plane, NULL);
+ intel_crtc_update_cursor(plane, NULL);
}
static void
@@ -13746,9 +13734,7 @@ intel_update_cursor_plane(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *state)
{
- struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-
- intel_crtc_update_cursor(&crtc->base, plane, state);
+ intel_crtc_update_cursor(plane, state);
}
static struct intel_plane *
@@ -13782,6 +13768,10 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
cursor->update_plane = intel_update_cursor_plane;
cursor->disable_plane = intel_disable_cursor_plane;
+ cursor->cursor.base = ~0;
+ cursor->cursor.cntl = ~0;
+ cursor->cursor.size = ~0;
+
ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
0, &intel_cursor_plane_funcs,
intel_cursor_formats,
@@ -13889,10 +13879,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
intel_crtc->pipe = pipe;
intel_crtc->plane = primary->plane;
- intel_crtc->cursor_base = ~0;
- intel_crtc->cursor_cntl = ~0;
- intel_crtc->cursor_size = ~0;
-
/* initialize shared scalers */
intel_crtc_init_scalers(intel_crtc, crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 54382f1f961c..d954fe458a61 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -757,11 +757,6 @@ struct intel_crtc {
int adjusted_x;
int adjusted_y;
- uint32_t cursor_addr;
- uint32_t cursor_cntl;
- uint32_t cursor_size;
- uint32_t cursor_base;
-
struct intel_crtc_state *config;
/* global reset count when the last flip was submitted */
@@ -802,6 +797,10 @@ struct intel_plane {
int max_downscale;
uint32_t frontbuffer_bit;
+ struct {
+ u32 base, cntl, size;
+ } cursor;
+
/*
* NOTE: Do not place new plane state fields here (e.g., when adding
* new plane properties). New runtime state should now be placed in
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc
2017-03-07 15:27 ` [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc ville.syrjala
@ 2017-03-07 22:33 ` Chris Wilson
0 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2017-03-07 22:33 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Mar 07, 2017 at 05:27:03PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Move cursor_base, cursor_cntl, and cursor_size from intel_crtc
> into intel_plane so that we don't need the crtc for cursor stuff
> so much.
>
> Also entirely nuke cursor_addr which IMO doesn't provide any benefit
> since it's not actually used by the cursor code itself. I'm not 100%
> sure what the SKL+ DDB is code is after by looking at cursor_addr so
> I just make it do its checks unconditionally. If that's not correct
> then we should likely replace it with somehting like
> plane_state->visible.
Looks ok to me, but I'm going to bow out of checking atomic state and
friends.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/10] drm/i915: Refactor CURPOS calculation
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (3 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 04/10] drm/i915: Clean up cursor junk from intel_crtc ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 21:49 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 06/10] drm/i915: Move cursor position and base handling into the platform specific functions ville.syrjala
` (5 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Move the CURPOS calculations to seprate function. This will allow
sharing the code between the 845/865 vs. others codepaths when we
otherwise split them apart.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 191685a37200..b6786fc1d883 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9175,6 +9175,28 @@ static u32 intel_cursor_base(struct intel_plane *plane,
return base;
}
+static u32 intel_cursor_position(struct intel_plane *plane,
+ const struct intel_plane_state *plane_state)
+{
+ int x = plane_state->base.crtc_x;
+ int y = plane_state->base.crtc_y;
+ u32 pos = 0;
+
+ if (x < 0) {
+ pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
+ x = -x;
+ }
+ pos |= x << CURSOR_X_SHIFT;
+
+ if (y < 0) {
+ pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
+ y = -y;
+ }
+ pos |= y << CURSOR_Y_SHIFT;
+
+ return pos;
+}
+
static void i845_update_cursor(struct intel_plane *plane, u32 base,
const struct intel_plane_state *plane_state)
{
@@ -9291,22 +9313,8 @@ static void intel_crtc_update_cursor(struct intel_plane *plane,
u32 pos = 0, base = 0;
if (plane_state) {
- int x = plane_state->base.crtc_x;
- int y = plane_state->base.crtc_y;
-
- if (x < 0) {
- pos |= CURSOR_POS_SIGN << CURSOR_X_SHIFT;
- x = -x;
- }
- pos |= x << CURSOR_X_SHIFT;
-
- if (y < 0) {
- pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
- y = -y;
- }
- pos |= y << CURSOR_Y_SHIFT;
-
base = intel_cursor_base(plane, plane_state);
+ pos = intel_cursor_position(plane, plane_state);
}
I915_WRITE(CURPOS(pipe), pos);
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 06/10] drm/i915: Move cursor position and base handling into the platform specific functions
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (4 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 05/10] drm/i915: Refactor CURPOS calculation ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:35 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 07/10] drm/i915: Drop useless posting reads from cursor commit ville.syrjala
` (4 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Supposedly on some platforms we can get extra atomicity guarantees for
CURPOS if we write it between the CURCNTR and CURBASE. Let's move the
CURPOS handling into the platform specific hooks to make the possible
without having to pass the calculated CURPOS around. And while at it,
do the same for the CURBASE to avoid passing that either.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++-------------------
1 file changed, 37 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b6786fc1d883..a3ff816fb152 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9197,11 +9197,12 @@ static u32 intel_cursor_position(struct intel_plane *plane,
return pos;
}
-static void i845_update_cursor(struct intel_plane *plane, u32 base,
+static void i845_update_cursor(struct intel_plane *plane,
+ const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
- uint32_t cntl = 0, size = 0;
+ u32 cntl = 0, base = 0, pos = 0, size = 0;
if (plane_state && plane_state->base.visible) {
unsigned int width = plane_state->base.crtc_w;
@@ -9227,6 +9228,9 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base,
CURSOR_STRIDE(stride);
size = (height << 12) | width;
+
+ base = intel_cursor_base(plane, plane_state);
+ pos = intel_cursor_position(plane, plane_state);
}
if (plane->cursor.cntl != 0 &&
@@ -9251,6 +9255,9 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base,
plane->cursor.size = size;
}
+ if (cntl)
+ I915_WRITE(CURPOS(PIPE_A), pos);
+
if (plane->cursor.cntl != cntl) {
I915_WRITE(CURCNTR(PIPE_A), cntl);
POSTING_READ(CURCNTR(PIPE_A));
@@ -9258,12 +9265,19 @@ static void i845_update_cursor(struct intel_plane *plane, u32 base,
}
}
-static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
+static void i845_disable_cursor(struct intel_plane *plane,
+ struct intel_crtc *crtc)
+{
+ i845_update_cursor(plane, NULL, NULL);
+}
+
+static void i9xx_update_cursor(struct intel_plane *plane,
+ const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
- uint32_t cntl = 0;
+ u32 cntl = 0, base = 0, pos = 0;
if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
@@ -9289,6 +9303,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
if (plane_state->base.rotation & DRM_ROTATE_180)
cntl |= CURSOR_ROTATE_180;
+
+ base = intel_cursor_base(plane, plane_state);
+ pos = intel_cursor_position(plane, plane_state);
}
if (plane->cursor.cntl != cntl) {
@@ -9297,6 +9314,9 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
plane->cursor.cntl = cntl;
}
+ if (cntl)
+ I915_WRITE(CURPOS(pipe), pos);
+
/* and commit changes on next vblank */
I915_WRITE(CURBASE(pipe), base);
POSTING_READ(CURBASE(pipe));
@@ -9304,25 +9324,10 @@ static void i9xx_update_cursor(struct intel_plane *plane, u32 base,
plane->cursor.base = base;
}
-/* If no-part of the cursor is visible on the framebuffer, then the GPU may hang... */
-static void intel_crtc_update_cursor(struct intel_plane *plane,
- const struct intel_plane_state *plane_state)
+static void i9xx_disable_cursor(struct intel_plane *plane,
+ struct intel_crtc *crtc)
{
- struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
- enum pipe pipe = plane->pipe;
- u32 pos = 0, base = 0;
-
- if (plane_state) {
- base = intel_cursor_base(plane, plane_state);
- pos = intel_cursor_position(plane, plane_state);
- }
-
- I915_WRITE(CURPOS(pipe), pos);
-
- if (IS_I845G(dev_priv) || IS_I865G(dev_priv))
- i845_update_cursor(plane, base, plane_state);
- else
- i9xx_update_cursor(plane, base, plane_state);
+ i9xx_update_cursor(plane, NULL, NULL);
}
static bool cursor_size_ok(struct drm_i915_private *dev_priv,
@@ -13730,23 +13735,9 @@ intel_check_cursor_plane(struct intel_plane *plane,
return 0;
}
-static void
-intel_disable_cursor_plane(struct intel_plane *plane,
- struct intel_crtc *crtc)
-{
- intel_crtc_update_cursor(plane, NULL);
-}
-
-static void
-intel_update_cursor_plane(struct intel_plane *plane,
- const struct intel_crtc_state *crtc_state,
- const struct intel_plane_state *state)
-{
- intel_crtc_update_cursor(plane, state);
-}
-
static struct intel_plane *
-intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
+intel_cursor_plane_create(struct drm_i915_private *dev_priv,
+ enum pipe pipe)
{
struct intel_plane *cursor = NULL;
struct intel_plane_state *state = NULL;
@@ -13773,8 +13764,14 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
cursor->id = PLANE_CURSOR;
cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
cursor->check_plane = intel_check_cursor_plane;
- cursor->update_plane = intel_update_cursor_plane;
- cursor->disable_plane = intel_disable_cursor_plane;
+
+ if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
+ cursor->update_plane = i845_update_cursor;
+ cursor->disable_plane = i845_disable_cursor;
+ } else {
+ cursor->update_plane = i9xx_update_cursor;
+ cursor->disable_plane = i9xx_disable_cursor;
+ }
cursor->cursor.base = ~0;
cursor->cursor.cntl = ~0;
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 07/10] drm/i915: Drop useless posting reads from cursor commit
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (5 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 06/10] drm/i915: Move cursor position and base handling into the platform specific functions ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:00 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants ville.syrjala
` (3 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
There should be no need to do posting reads between all the cursor
register accessess. Let's just drop them.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a3ff816fb152..222f54ffd113 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9241,28 +9241,26 @@ static void i845_update_cursor(struct intel_plane *plane,
* whilst the cursor is disabled.
*/
I915_WRITE(CURCNTR(PIPE_A), 0);
- POSTING_READ(CURCNTR(PIPE_A));
plane->cursor.cntl = 0;
}
- if (plane->cursor.base != base) {
+ if (plane->cursor.base != base)
I915_WRITE(CURBASE(PIPE_A), base);
- plane->cursor.base = base;
- }
- if (plane->cursor.size != size) {
+ if (plane->cursor.size != size)
I915_WRITE(CURSIZE, size);
- plane->cursor.size = size;
- }
if (cntl)
I915_WRITE(CURPOS(PIPE_A), pos);
- if (plane->cursor.cntl != cntl) {
+ if (plane->cursor.cntl != cntl)
I915_WRITE(CURCNTR(PIPE_A), cntl);
- POSTING_READ(CURCNTR(PIPE_A));
- plane->cursor.cntl = cntl;
- }
+
+ POSTING_READ(CURCNTR(PIPE_A));
+
+ plane->cursor.cntl = cntl;
+ plane->cursor.base = base;
+ plane->cursor.size = size;
}
static void i845_disable_cursor(struct intel_plane *plane,
@@ -9308,19 +9306,19 @@ static void i9xx_update_cursor(struct intel_plane *plane,
pos = intel_cursor_position(plane, plane_state);
}
- if (plane->cursor.cntl != cntl) {
+ if (plane->cursor.cntl != cntl)
I915_WRITE(CURCNTR(pipe), cntl);
- POSTING_READ(CURCNTR(pipe));
- plane->cursor.cntl = cntl;
- }
if (cntl)
I915_WRITE(CURPOS(pipe), pos);
- /* and commit changes on next vblank */
- I915_WRITE(CURBASE(pipe), base);
+ if (plane->cursor.cntl != cntl ||
+ plane->cursor.base != base)
+ I915_WRITE(CURBASE(pipe), base);
+
POSTING_READ(CURBASE(pipe));
+ plane->cursor.cntl = cntl;
plane->cursor.base = base;
}
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (6 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 07/10] drm/i915: Drop useless posting reads from cursor commit ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:24 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code ville.syrjala
` (2 subsequent siblings)
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The 845/865 and 830/855/9xx+ style cursor don't have that
much in common with each other, so let's just split the
.check_plane() hook into two variants as well.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 232 ++++++++++++++++++++++-------------
1 file changed, 145 insertions(+), 87 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 222f54ffd113..41cbaee66f1b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9269,6 +9269,74 @@ static void i845_disable_cursor(struct intel_plane *plane,
i845_update_cursor(plane, NULL, NULL);
}
+static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
+{
+ struct drm_i915_private *dev_priv =
+ to_i915(plane_state->base.plane->dev);
+ int width = plane_state->base.crtc_w;
+ int height = plane_state->base.crtc_h;
+
+ if (width == 0 || height == 0)
+ return false;
+
+ /*
+ * 845g/865g are only limited by the width of their cursors,
+ * the height is arbitrary up to the precision of the register.
+ */
+ if ((width & 63) != 0)
+ return false;
+
+ if (width > (IS_I845G(dev_priv) ? 64 : 512))
+ return false;
+
+ if (height > 1023)
+ return false;
+
+ return true;
+}
+
+static int i845_check_cursor(struct intel_plane *plane,
+ struct intel_crtc_state *crtc_state,
+ struct intel_plane_state *state)
+{
+ struct drm_framebuffer *fb = state->base.fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ unsigned stride;
+ int ret;
+
+ ret = drm_plane_helper_check_state(&state->base,
+ &state->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
+ if (ret)
+ return ret;
+
+ /* if we want to turn off the cursor ignore width and height */
+ if (!obj)
+ return 0;
+
+ /* Check for which cursor types we support */
+ if (!i845_cursor_size_ok(state)) {
+ DRM_DEBUG("Cursor dimension %dx%d not supported\n",
+ state->base.crtc_w, state->base.crtc_h);
+ return -EINVAL;
+ }
+
+ stride = roundup_pow_of_two(state->base.crtc_w) * 4;
+ if (obj->base.size < stride * state->base.crtc_h) {
+ DRM_DEBUG_KMS("buffer is too small\n");
+ return -ENOMEM;
+ }
+
+ if (fb->modifier != DRM_FORMAT_MOD_NONE) {
+ DRM_DEBUG_KMS("cursor cannot be tiled\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void i9xx_update_cursor(struct intel_plane *plane,
const struct intel_crtc_state *crtc_state,
const struct intel_plane_state *plane_state)
@@ -9328,41 +9396,92 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
i9xx_update_cursor(plane, NULL, NULL);
}
-static bool cursor_size_ok(struct drm_i915_private *dev_priv,
- uint32_t width, uint32_t height)
+static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
{
+ struct drm_i915_private *dev_priv =
+ to_i915(plane_state->base.plane->dev);
+ int width = plane_state->base.crtc_w;
+ int height = plane_state->base.crtc_h;
+
if (width == 0 || height == 0)
return false;
/*
- * 845g/865g are special in that they are only limited by
- * the width of their cursors, the height is arbitrary up to
- * the precision of the register. Everything else requires
- * square cursors, limited to a few power-of-two sizes.
+ * Cursors are limited to a few power-of-two
+ * sizes, and they must be square.
*/
- if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
- if ((width & 63) != 0)
+ switch (width | height) {
+ case 256:
+ case 128:
+ if (IS_GEN2(dev_priv))
return false;
+ case 64:
+ break;
+ default:
+ return false;
+ }
- if (width > (IS_I845G(dev_priv) ? 64 : 512))
- return false;
+ return true;
+}
- if (height > 1023)
- return false;
- } else {
- switch (width | height) {
- case 256:
- case 128:
- if (IS_GEN2(dev_priv))
- return false;
- case 64:
- break;
- default:
- return false;
- }
+static int i9xx_check_cursor(struct intel_plane *plane,
+ struct intel_crtc_state *crtc_state,
+ struct intel_plane_state *state)
+{
+ struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
+ struct drm_framebuffer *fb = state->base.fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ enum pipe pipe = plane->pipe;
+ unsigned stride;
+ int ret;
+
+ ret = drm_plane_helper_check_state(&state->base,
+ &state->clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ true, true);
+ if (ret)
+ return ret;
+
+ /* if we want to turn off the cursor ignore width and height */
+ if (!obj)
+ return 0;
+
+ /* Check for which cursor types we support */
+ if (!i9xx_cursor_size_ok(state)) {
+ DRM_DEBUG("Cursor dimension %dx%d not supported\n",
+ state->base.crtc_w, state->base.crtc_h);
+ return -EINVAL;
}
- return true;
+ stride = roundup_pow_of_two(state->base.crtc_w) * 4;
+ if (obj->base.size < stride * state->base.crtc_h) {
+ DRM_DEBUG_KMS("buffer is too small\n");
+ return -ENOMEM;
+ }
+
+ if (fb->modifier != DRM_FORMAT_MOD_NONE) {
+ DRM_DEBUG_KMS("cursor cannot be tiled\n");
+ return -EINVAL;
+ }
+
+ /*
+ * There's something wrong with the cursor on CHV pipe C.
+ * If it straddles the left edge of the screen then
+ * moving it away from the edge or disabling it often
+ * results in a pipe underrun, and often that can lead to
+ * dead pipe (constant underrun reported, and it scans
+ * out just a solid color). To recover from that, the
+ * display power well must be turned off and on again.
+ * Refuse the put the cursor into that compromised position.
+ */
+ if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
+ state->base.visible && state->base.crtc_x < 0) {
+ DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
+ return -EINVAL;
+ }
+
+ return 0;
}
/* VESA 640x480x72Hz mode to set on the pipe */
@@ -13671,68 +13790,6 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe)
return ERR_PTR(ret);
}
-static int
-intel_check_cursor_plane(struct intel_plane *plane,
- struct intel_crtc_state *crtc_state,
- struct intel_plane_state *state)
-{
- struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
- struct drm_framebuffer *fb = state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- enum pipe pipe = plane->pipe;
- unsigned stride;
- int ret;
-
- ret = drm_plane_helper_check_state(&state->base,
- &state->clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- true, true);
- if (ret)
- return ret;
-
- /* if we want to turn off the cursor ignore width and height */
- if (!obj)
- return 0;
-
- /* Check for which cursor types we support */
- if (!cursor_size_ok(dev_priv, state->base.crtc_w,
- state->base.crtc_h)) {
- DRM_DEBUG("Cursor dimension %dx%d not supported\n",
- state->base.crtc_w, state->base.crtc_h);
- return -EINVAL;
- }
-
- stride = roundup_pow_of_two(state->base.crtc_w) * 4;
- if (obj->base.size < stride * state->base.crtc_h) {
- DRM_DEBUG_KMS("buffer is too small\n");
- return -ENOMEM;
- }
-
- if (fb->modifier != DRM_FORMAT_MOD_NONE) {
- DRM_DEBUG_KMS("cursor cannot be tiled\n");
- return -EINVAL;
- }
-
- /*
- * There's something wrong with the cursor on CHV pipe C.
- * If it straddles the left edge of the screen then
- * moving it away from the edge or disabling it often
- * results in a pipe underrun, and often that can lead to
- * dead pipe (constant underrun reported, and it scans
- * out just a solid color). To recover from that, the
- * display power well must be turned off and on again.
- * Refuse the put the cursor into that compromised position.
- */
- if (IS_CHERRYVIEW(dev_priv) && pipe == PIPE_C &&
- state->base.visible && state->base.crtc_x < 0) {
- DRM_DEBUG_KMS("CHV cursor C not allowed to straddle the left screen edge\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
static struct intel_plane *
intel_cursor_plane_create(struct drm_i915_private *dev_priv,
enum pipe pipe)
@@ -13761,14 +13818,15 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
cursor->plane = pipe;
cursor->id = PLANE_CURSOR;
cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
- cursor->check_plane = intel_check_cursor_plane;
if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
cursor->update_plane = i845_update_cursor;
cursor->disable_plane = i845_disable_cursor;
+ cursor->check_plane = i845_check_cursor;
} else {
cursor->update_plane = i9xx_update_cursor;
cursor->disable_plane = i9xx_disable_cursor;
+ cursor->check_plane = i9xx_check_cursor;
}
cursor->cursor.base = ~0;
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants
2017-03-07 15:27 ` [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants ville.syrjala
@ 2017-03-07 22:24 ` Chris Wilson
2017-03-08 10:52 ` Ville Syrjälä
0 siblings, 1 reply; 26+ messages in thread
From: Chris Wilson @ 2017-03-07 22:24 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Mar 07, 2017 at 05:27:07PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The 845/865 and 830/855/9xx+ style cursor don't have that
> much in common with each other, so let's just split the
> .check_plane() hook into two variants as well.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 232 ++++++++++++++++++++++-------------
> 1 file changed, 145 insertions(+), 87 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 222f54ffd113..41cbaee66f1b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9269,6 +9269,74 @@ static void i845_disable_cursor(struct intel_plane *plane,
> i845_update_cursor(plane, NULL, NULL);
> }
>
> +static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
> +{
> + struct drm_i915_private *dev_priv =
> + to_i915(plane_state->base.plane->dev);
> + int width = plane_state->base.crtc_w;
> + int height = plane_state->base.crtc_h;
> +
> + if (width == 0 || height == 0)
> + return false;
> +
> + /*
> + * 845g/865g are only limited by the width of their cursors,
> + * the height is arbitrary up to the precision of the register.
> + */
> + if ((width & 63) != 0)
if (!IS_ALIGNED(width, 64)) ?
> + return false;
> +
> + if (width > (IS_I845G(dev_priv) ? 64 : 512))
> + return false;
> +
> + if (height > 1023)
> + return false;
> +
> + return true;
> +}
> static void i9xx_update_cursor(struct intel_plane *plane,
> const struct intel_crtc_state *crtc_state,
> const struct intel_plane_state *plane_state)
> @@ -9328,41 +9396,92 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
> i9xx_update_cursor(plane, NULL, NULL);
> }
>
> -static bool cursor_size_ok(struct drm_i915_private *dev_priv,
> - uint32_t width, uint32_t height)
> +static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
> {
> + struct drm_i915_private *dev_priv =
> + to_i915(plane_state->base.plane->dev);
> + int width = plane_state->base.crtc_w;
> + int height = plane_state->base.crtc_h;
> +
> if (width == 0 || height == 0)
> return false;
>
> /*
> - * 845g/865g are special in that they are only limited by
> - * the width of their cursors, the height is arbitrary up to
> - * the precision of the register. Everything else requires
> - * square cursors, limited to a few power-of-two sizes.
> + * Cursors are limited to a few power-of-two
> + * sizes, and they must be square.
> */
> - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
> - if ((width & 63) != 0)
> + switch (width | height) {
> + case 256:
> + case 128:
> + if (IS_GEN2(dev_priv))
> return false;
> + case 64:
> + break;
> + default:
> + return false;
Checks out ok.
> + }
There's still quite a bit of boilerplate duplication between the two
check_plane functions. No chance for some sharing? I presume their is
also an ulterior motive for the split in later patches. That would be
worth mentioning in the changelog to quell some of the doubts over
duplication.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants
2017-03-07 22:24 ` Chris Wilson
@ 2017-03-08 10:52 ` Ville Syrjälä
0 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjälä @ 2017-03-08 10:52 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Tue, Mar 07, 2017 at 10:24:21PM +0000, Chris Wilson wrote:
> On Tue, Mar 07, 2017 at 05:27:07PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The 845/865 and 830/855/9xx+ style cursor don't have that
> > much in common with each other, so let's just split the
> > .check_plane() hook into two variants as well.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 232 ++++++++++++++++++++++-------------
> > 1 file changed, 145 insertions(+), 87 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 222f54ffd113..41cbaee66f1b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9269,6 +9269,74 @@ static void i845_disable_cursor(struct intel_plane *plane,
> > i845_update_cursor(plane, NULL, NULL);
> > }
> >
> > +static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
> > +{
> > + struct drm_i915_private *dev_priv =
> > + to_i915(plane_state->base.plane->dev);
> > + int width = plane_state->base.crtc_w;
> > + int height = plane_state->base.crtc_h;
> > +
> > + if (width == 0 || height == 0)
> > + return false;
> > +
> > + /*
> > + * 845g/865g are only limited by the width of their cursors,
> > + * the height is arbitrary up to the precision of the register.
> > + */
> > + if ((width & 63) != 0)
>
> if (!IS_ALIGNED(width, 64)) ?
Why not.
>
> > + return false;
> > +
> > + if (width > (IS_I845G(dev_priv) ? 64 : 512))
> > + return false;
> > +
> > + if (height > 1023)
> > + return false;
> > +
> > + return true;
> > +}
>
> > static void i9xx_update_cursor(struct intel_plane *plane,
> > const struct intel_crtc_state *crtc_state,
> > const struct intel_plane_state *plane_state)
> > @@ -9328,41 +9396,92 @@ static void i9xx_disable_cursor(struct intel_plane *plane,
> > i9xx_update_cursor(plane, NULL, NULL);
> > }
> >
> > -static bool cursor_size_ok(struct drm_i915_private *dev_priv,
> > - uint32_t width, uint32_t height)
> > +static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
> > {
> > + struct drm_i915_private *dev_priv =
> > + to_i915(plane_state->base.plane->dev);
> > + int width = plane_state->base.crtc_w;
> > + int height = plane_state->base.crtc_h;
> > +
> > if (width == 0 || height == 0)
> > return false;
> >
> > /*
> > - * 845g/865g are special in that they are only limited by
> > - * the width of their cursors, the height is arbitrary up to
> > - * the precision of the register. Everything else requires
> > - * square cursors, limited to a few power-of-two sizes.
> > + * Cursors are limited to a few power-of-two
> > + * sizes, and they must be square.
> > */
> > - if (IS_I845G(dev_priv) || IS_I865G(dev_priv)) {
> > - if ((width & 63) != 0)
> > + switch (width | height) {
> > + case 256:
> > + case 128:
> > + if (IS_GEN2(dev_priv))
> > return false;
> > + case 64:
> > + break;
> > + default:
> > + return false;
>
> Checks out ok.
>
> > + }
>
> There's still quite a bit of boilerplate duplication between the two
> check_plane functions. No chance for some sharing? I presume their is
> also an ulterior motive for the split in later patches. That would be
> worth mentioning in the changelog to quell some of the doubts over
> duplication.
I think in the end we're left with the drm_plane_helper_check_state()
call and the modifier check. Those could be done back to back and moved
to a common place. Although I'm sort of hoping the modifier check can
be made to vanish once we get the getplane2 stuff and the driver provides
the core with an explicit list of acceptable modifiers for each plane.
So we might be left with just the check_state() call.
OTOH we're actually missing any and all checks for the src coordinates.
The cursor plane can't pan around freely, so we should add some checks
for that, and putting those into a common place migth be the right
thing. I'm not sure if we should just say you can't pan at all, or
that the panning must happen in whatever chunks CURBASE allows.
Obviously on most platforms that would still only allow vertical
panning in units of several lines. Should be pretty trivial to achieve
that with a call to the compute_tile_offset() thing and a check to make
sure the leftover x/y coordinates are 0.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (7 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 08/10] drm/i915: Split cursor check_plane into i845 and i9xx variants ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:25 ` Chris Wilson
2017-03-07 15:27 ` [PATCH 10/10] drm/i915: Support variable cursor height on ivb+ ville.syrjala
2017-03-07 16:18 ` ✓ Fi.CI.BAT: success for drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ Patchwork
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The cursor code currently ignores fb->pitches[0] (except when creating
the fb itself), and just uses the cursor_width*4 as the stride. Let's
make sure fb->pitches[0] actually matches what we expect it to be.
We can also relax the stride vs. cursor width relationship on 845/865
since the stride is programmed separately. The only constraint is that
width*cpp doesn't exceed the stride, and that's already been checked
by the core since it makes sure the entire plane fits within the fb.
We can also drop the bo size check as that's already checked when
we create the fb. That is the fb is guaranteed to fit within the bo.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 41cbaee66f1b..17362dc9f438 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9300,8 +9300,6 @@ static int i845_check_cursor(struct intel_plane *plane,
struct intel_plane_state *state)
{
struct drm_framebuffer *fb = state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- unsigned stride;
int ret;
ret = drm_plane_helper_check_state(&state->base,
@@ -9313,7 +9311,7 @@ static int i845_check_cursor(struct intel_plane *plane,
return ret;
/* if we want to turn off the cursor ignore width and height */
- if (!obj)
+ if (!fb)
return 0;
/* Check for which cursor types we support */
@@ -9323,10 +9321,16 @@ static int i845_check_cursor(struct intel_plane *plane,
return -EINVAL;
}
- stride = roundup_pow_of_two(state->base.crtc_w) * 4;
- if (obj->base.size < stride * state->base.crtc_h) {
- DRM_DEBUG_KMS("buffer is too small\n");
- return -ENOMEM;
+ switch (fb->pitches[0]) {
+ case 256:
+ case 512:
+ case 1024:
+ case 2048:
+ break;
+ default:
+ DRM_DEBUG_KMS("Invalid cursor stride (%u)\n",
+ fb->pitches[0]);
+ return -EINVAL;
}
if (fb->modifier != DRM_FORMAT_MOD_NONE) {
@@ -9430,9 +9434,7 @@ static int i9xx_check_cursor(struct intel_plane *plane,
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
struct drm_framebuffer *fb = state->base.fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
enum pipe pipe = plane->pipe;
- unsigned stride;
int ret;
ret = drm_plane_helper_check_state(&state->base,
@@ -9444,7 +9446,7 @@ static int i9xx_check_cursor(struct intel_plane *plane,
return ret;
/* if we want to turn off the cursor ignore width and height */
- if (!obj)
+ if (!fb)
return 0;
/* Check for which cursor types we support */
@@ -9454,10 +9456,10 @@ static int i9xx_check_cursor(struct intel_plane *plane,
return -EINVAL;
}
- stride = roundup_pow_of_two(state->base.crtc_w) * 4;
- if (obj->base.size < stride * state->base.crtc_h) {
- DRM_DEBUG_KMS("buffer is too small\n");
- return -ENOMEM;
+ if (fb->pitches[0] != state->base.crtc_w * fb->format->cpp[0]) {
+ DRM_DEBUG_KMS("Invalid cursor stride (%u) (cursor width %d)\n",
+ fb->pitches[0], state->base.crtc_w);
+ return -EINVAL;
}
if (fb->modifier != DRM_FORMAT_MOD_NONE) {
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code
2017-03-07 15:27 ` [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code ville.syrjala
@ 2017-03-07 22:25 ` Chris Wilson
0 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2017-03-07 22:25 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Mar 07, 2017 at 05:27:08PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The cursor code currently ignores fb->pitches[0] (except when creating
> the fb itself), and just uses the cursor_width*4 as the stride. Let's
> make sure fb->pitches[0] actually matches what we expect it to be.
>
> We can also relax the stride vs. cursor width relationship on 845/865
> since the stride is programmed separately. The only constraint is that
> width*cpp doesn't exceed the stride, and that's already been checked
> by the core since it makes sure the entire plane fits within the fb.
>
> We can also drop the bo size check as that's already checked when
> we create the fb. That is the fb is guaranteed to fit within the bo.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (8 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 09/10] drm/i915: Use fb->pitches[0] in cursor code ville.syrjala
@ 2017-03-07 15:27 ` ville.syrjala
2017-03-07 22:32 ` Chris Wilson
2017-03-07 16:18 ` ✓ Fi.CI.BAT: success for drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ Patchwork
10 siblings, 1 reply; 26+ messages in thread
From: ville.syrjala @ 2017-03-07 15:27 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
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. CUR_FBC_CTL can't be used when the cursor
is rotated.
Commandeer the otherwise unused cursor->cursor.size to track the
current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
writes, and to notice when we need to arm the update via CURBASE if
just CUR_FBC_CTL changes.
v2: Reverse the gen check to make it sane
v3: Only enable CUR_FBC_CTL when cursor is enabled, adapt to
earlier code changes which means we now actually turn off
the cursor when we're supposed to unlike v2
v4: Add a comment about rotation vs. CUR_FBC_CTL,
rebase due to 'dirty' (Chris)
v5: Rebase to the atomic world
Handle 180 degree rotation
Add HAS_CUR_FBC()
v6: Rebase
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_reg.h | 5 ++++-
drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++-------
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7ec6636e2a0..c96b667d6e6e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2900,6 +2900,7 @@ intel_info(const struct drm_i915_private *dev_priv)
#define HAS_FW_BLC(dev_priv) (INTEL_GEN(dev_priv) > 2)
#define HAS_PIPE_CXSR(dev_priv) ((dev_priv)->info.has_pipe_cxsr)
#define HAS_FBC(dev_priv) ((dev_priv)->info.has_fbc)
+#define HAS_CUR_FBC(dev_priv) (!HAS_GMCH_DISPLAY(dev_priv) && INTEL_INFO(dev_priv)->gen >= 7)
#define HAS_IPS(dev_priv) (IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 07cae03e8727..cfcb139e5952 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5449,7 +5449,9 @@ enum {
#define CURSOR_POS_SIGN 0x8000
#define CURSOR_X_SHIFT 0
#define CURSOR_Y_SHIFT 16
-#define CURSIZE _MMIO(0x700a0)
+#define CURSIZE _MMIO(0x700a0) /* 845/865 */
+#define _CUR_FBC_CTL_A 0x700a0 /* ivb+ */
+#define CUR_FBC_CTL_EN (1 << 31)
#define _CURBCNTR 0x700c0
#define _CURBBASE 0x700c4
#define _CURBPOS 0x700c8
@@ -5465,6 +5467,7 @@ enum {
#define CURCNTR(pipe) _CURSOR2(pipe, _CURACNTR)
#define CURBASE(pipe) _CURSOR2(pipe, _CURABASE)
#define CURPOS(pipe) _CURSOR2(pipe, _CURAPOS)
+#define CUR_FBC_CTL(pipe) _CURSOR2(pipe, _CUR_FBC_CTL_A)
#define CURSOR_A_OFFSET 0x70080
#define CURSOR_B_OFFSET 0x700c0
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 17362dc9f438..13b31e929631 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9347,7 +9347,7 @@ static void i9xx_update_cursor(struct intel_plane *plane,
{
struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
enum pipe pipe = plane->pipe;
- u32 cntl = 0, base = 0, pos = 0;
+ u32 cntl = 0, base = 0, pos = 0, size = 0;
if (plane_state && plane_state->base.visible) {
cntl = MCURSOR_GAMMA_ENABLE;
@@ -9376,15 +9376,22 @@ static void i9xx_update_cursor(struct intel_plane *plane,
base = intel_cursor_base(plane, plane_state);
pos = intel_cursor_position(plane, plane_state);
+
+ if (plane_state->base.crtc_h != plane_state->base.crtc_w)
+ size = CUR_FBC_CTL_EN | (plane_state->base.crtc_h - 1);
}
if (plane->cursor.cntl != cntl)
I915_WRITE(CURCNTR(pipe), cntl);
+ if (plane->cursor.size != size)
+ I915_WRITE(CUR_FBC_CTL(pipe), size);
+
if (cntl)
I915_WRITE(CURPOS(pipe), pos);
if (plane->cursor.cntl != cntl ||
+ plane->cursor.size != size ||
plane->cursor.base != base)
I915_WRITE(CURBASE(pipe), base);
@@ -9392,6 +9399,7 @@ static void i9xx_update_cursor(struct intel_plane *plane,
plane->cursor.cntl = cntl;
plane->cursor.base = base;
+ plane->cursor.size = size;
}
static void i9xx_disable_cursor(struct intel_plane *plane,
@@ -9410,11 +9418,8 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
if (width == 0 || height == 0)
return false;
- /*
- * Cursors are limited to a few power-of-two
- * sizes, and they must be square.
- */
- switch (width | height) {
+ /* Cursor width is limited to a few power-of-two sizes */
+ switch (width) {
case 256:
case 128:
if (IS_GEN2(dev_priv))
@@ -9425,6 +9430,21 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
return false;
}
+ /*
+ * IVB+ have CUR_FBC_CTL which allows an arbitrary cursor
+ * height from 8 lines up to the cursor width, when the
+ * cursor is not rotated. Everything else requires square
+ * cursors.
+ */
+ if (HAS_CUR_FBC(dev_priv) &&
+ plane_state->base.rotation & DRM_ROTATE_0) {
+ if (height < 8 || height > width)
+ return false;
+ } else {
+ if (height != width)
+ return false;
+ }
+
return true;
}
@@ -13833,7 +13853,9 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
cursor->cursor.base = ~0;
cursor->cursor.cntl = ~0;
- cursor->cursor.size = ~0;
+
+ if (IS_I845G(dev_priv) || IS_I865G(dev_priv) || HAS_CUR_FBC(dev_priv))
+ cursor->cursor.size = ~0;
ret = drm_universal_plane_init(&dev_priv->drm, &cursor->base,
0, &intel_cursor_plane_funcs,
--
2.10.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
2017-03-07 15:27 ` [PATCH 10/10] drm/i915: Support variable cursor height on ivb+ ville.syrjala
@ 2017-03-07 22:32 ` Chris Wilson
2017-03-08 10:40 ` Ville Syrjälä
2017-03-08 10:45 ` Daniel Vetter
0 siblings, 2 replies; 26+ messages in thread
From: Chris Wilson @ 2017-03-07 22:32 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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. CUR_FBC_CTL can't be used when the cursor
> is rotated.
>
> Commandeer the otherwise unused cursor->cursor.size to track the
> current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
> writes, and to notice when we need to arm the update via CURBASE if
> just CUR_FBC_CTL changes.
For userspace to discover this, they should just use trial and error
during startup (using the legacy SetCursor)? Though the easiest way
would cause the cursor to flicker - just hope the cursor is
off/invisible on startup.
Code makes a lot of sense after all the refactoring, but I'll leave the
register checking to somebody else unless they are lazier than I am.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
2017-03-07 22:32 ` Chris Wilson
@ 2017-03-08 10:40 ` Ville Syrjälä
2017-03-08 11:00 ` Chris Wilson
2017-03-08 10:45 ` Daniel Vetter
1 sibling, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2017-03-08 10:40 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote:
> On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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. CUR_FBC_CTL can't be used when the cursor
> > is rotated.
> >
> > Commandeer the otherwise unused cursor->cursor.size to track the
> > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
> > writes, and to notice when we need to arm the update via CURBASE if
> > just CUR_FBC_CTL changes.
>
> For userspace to discover this, they should just use trial and error
> during startup (using the legacy SetCursor)? Though the easiest way
> would cause the cursor to flicker - just hope the cursor is
> off/invisible on startup.
I don't have any decent answer for how to discover this feature. Atomic
would allow the trial and error approach without flickers. Otherwise I
guess one could defer the check until the cursor size actually changes.
Although maybe that won't really work for Xorg. ISTR it wants to know
things about the cursor size upfront?
> Code makes a lot of sense after all the refactoring, but I'll leave the
> register checking to somebody else unless they are lazier than I am.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
2017-03-08 10:40 ` Ville Syrjälä
@ 2017-03-08 11:00 ` Chris Wilson
0 siblings, 0 replies; 26+ messages in thread
From: Chris Wilson @ 2017-03-08 11:00 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On Wed, Mar 08, 2017 at 12:40:53PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote:
> > On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > 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. CUR_FBC_CTL can't be used when the cursor
> > > is rotated.
> > >
> > > Commandeer the otherwise unused cursor->cursor.size to track the
> > > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
> > > writes, and to notice when we need to arm the update via CURBASE if
> > > just CUR_FBC_CTL changes.
> >
> > For userspace to discover this, they should just use trial and error
> > during startup (using the legacy SetCursor)? Though the easiest way
> > would cause the cursor to flicker - just hope the cursor is
> > off/invisible on startup.
>
> I don't have any decent answer for how to discover this feature. Atomic
> would allow the trial and error approach without flickers. Otherwise I
> guess one could defer the check until the cursor size actually changes.
> Although maybe that won't really work for Xorg. ISTR it wants to know
> things about the cursor size upfront?
We resize the cursor on the fly, so we could do a trial-and-error after
a resize. But yes, the old cursor code used a static one-size fits all.
(gem bo alloc is signalsafe, but malloc isn't -- so we just need to make
sure we have a spare slot to create a new cursor in.)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 10/10] drm/i915: Support variable cursor height on ivb+
2017-03-07 22:32 ` Chris Wilson
2017-03-08 10:40 ` Ville Syrjälä
@ 2017-03-08 10:45 ` Daniel Vetter
1 sibling, 0 replies; 26+ messages in thread
From: Daniel Vetter @ 2017-03-08 10:45 UTC (permalink / raw)
To: Chris Wilson, ville.syrjala, intel-gfx
On Tue, Mar 07, 2017 at 10:32:14PM +0000, Chris Wilson wrote:
> On Tue, Mar 07, 2017 at 05:27:09PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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. CUR_FBC_CTL can't be used when the cursor
> > is rotated.
> >
> > Commandeer the otherwise unused cursor->cursor.size to track the
> > current value of CUR_FBC_CTL to optimize away redundant CUR_FBC_CTL
> > writes, and to notice when we need to arm the update via CURBASE if
> > just CUR_FBC_CTL changes.
>
> For userspace to discover this, they should just use trial and error
> during startup (using the legacy SetCursor)? Though the easiest way
> would cause the cursor to flicker - just hope the cursor is
> off/invisible on startup.
>
> Code makes a lot of sense after all the refactoring, but I'll leave the
> register checking to somebody else unless they are lazier than I am.
TEST_ONLY atomic commits on the cursor plane. That's pretty much how it's
meant to be used (although no one yet figured out how to tell generic
userspace about fancy constraints like this one here).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+
2017-03-07 15:26 [PATCH 00/10] drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+ ville.syrjala
` (9 preceding siblings ...)
2017-03-07 15:27 ` [PATCH 10/10] drm/i915: Support variable cursor height on ivb+ ville.syrjala
@ 2017-03-07 16:18 ` Patchwork
10 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2017-03-07 16:18 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+
URL : https://patchwork.freedesktop.org/series/20835/
State : success
== Summary ==
Series 20835v1 drm/i915: Cursor code cleanup and cursor "FBC" support for IVB+
https://patchwork.freedesktop.org/api/1.0/series/20835/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (fi-byt-n2820)
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 475s
fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 609s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 533s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 603s
fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 504s
fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 501s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 433s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 435s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 439s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 511s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 474s
fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 476s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 507s
fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 595s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 501s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 561s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 556s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 423s
ba93250bd451d62c73db7ed39242a625730424f2 drm-tip: 2017y-03m-07d-13h-23m-43s UTC integration manifest
b8e5718 drm/i915: Support variable cursor height on ivb+
bb9c59d drm/i915: Use fb->pitches[0] in cursor code
cb8bd0f drm/i915: Split cursor check_plane into i845 and i9xx variants
647a321 drm/i915: Drop useless posting reads from cursor commit
0fe5163 drm/i915: Move cursor position and base handling into the platform specific functions
7f5b846 drm/i915: Refactor CURPOS calculation
395e755 drm/i915: Clean up cursor junk from intel_crtc
8b8b73c drm/i915: Refactor CURBASE calculation
f8c341da drm/i915: Pass intel_plane and intel_crtc to plane hooks
dd96657 drm/i915: Parametrize cursor/primary pipe select bits
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4086/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 26+ messages in thread