intel_plane->plane has inconsistent meanings across the driver:
* For primary planes, intel_plane->plane is usually the pipe number
(except for old platforms where it had to be swapped for FBC
reasons).
* For sprite planes, intel_plane->plane represents the sprite index for
the specific CRTC the sprite belongs to (0, 1, 2, ...)
* For cursor planes, intel_plane->plane is again the pipe number,
but without the FBC swapping on old platforms.
This overloading of the field can be quite confusing and easily leads to
bugs due to differences in how the hardware actually gets programmed
(e.g., SKL code needs to use p->plane + 1 because the hardware expects
universal plane ID's that include the primary, whereas VLV code needs to
use just p->plane because hardware expects a sprite index that does not
include the primary).
Let's try to clean up this situation by giving intel_plane->plane a
consistent meaning across all plane types, and then update all uses
across driver code to use macros to interpret it properly. The
following macros should be used in the code where we previously accessed
p->plane and will do additional plane type checking to help prevent
misuse:
* I915_UNIVERSAL_NUM(p) --- Universal plane ID (primary = 0, sprites
start from 1); intended for use in gen9+ code.
* I915_I915_PRIMARY_NUM(p) --- Primary plane ID for pre-gen9 platforms;
determines whether FBC-related swapping is needed or not.
* I915_SPRITE_NUM(p) --- Sprite plane ID (first sprite = 0); intended
for use in pre-gen9 code.
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 24 ++++++++++++++++++------
drivers/gpu/drm/i915/intel_display.c | 12 ++++--------
drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
drivers/gpu/drm/i915/intel_sprite.c | 13 +++++++------
4 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3bf8a9b..132fe53 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -131,22 +131,34 @@ enum transcoder {
#define transcoder_name(t) ((t) + 'A')
/*
- * This is the maximum (across all platforms) number of planes (primary +
- * sprites) that can be active at the same time on one pipe.
- *
- * This value doesn't count the cursor plane.
+ * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
+ * number of planes per CRTC. Not all platforms really have this many planes,
+ * which means some arrays of size I915_MAX_PLANES may have unused entries
+ * between the topmost sprite plane and the cursor plane.
*/
-#define I915_MAX_PLANES 4
-
enum plane {
PLANE_A = 0,
+ PLANE_PRIMARY = PLANE_A,
PLANE_B,
PLANE_C,
+ PLANE_CURSOR,
+ I915_MAX_PLANES,
};
#define plane_name(p) ((p) + 'A')
#define sprite_name(p, s) ((p) * INTEL_INFO(dev)->num_sprites[(p)] + (s) + 'A')
+#define I915_UNIVERSAL_NUM(p) ( \
+ WARN_ON(p->base.type == DRM_PLANE_TYPE_CURSOR), \
+ p->plane)
+#define I915_PRIMARY_NUM(p) ( \
+ WARN_ON(p->base.type != DRM_PLANE_TYPE_PRIMARY), \
+ (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) ? \
+ !p->pipe : p->pipe)
+#define I915_SPRITE_NUM(p) ( \
+ WARN_ON(p->base.type != DRM_PLANE_TYPE_OVERLAY), \
+ p->plane - 1)
+
enum port {
PORT_A = 0,
PORT_B,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fc00867..dab0b71 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11976,16 +11976,14 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d "
"disabled, scaler_id = %d\n",
plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
- plane->base.id, intel_plane->pipe,
- (crtc->base.primary == plane) ? 0 : intel_plane->plane + 1,
+ plane->base.id, intel_plane->pipe, intel_plane->plane,
drm_plane_index(plane), state->scaler_id);
continue;
}
DRM_DEBUG_KMS("%s PLANE:%d plane: %u.%u idx: %d enabled",
plane->type == DRM_PLANE_TYPE_CURSOR ? "CURSOR" : "STANDARD",
- plane->base.id, intel_plane->pipe,
- crtc->base.primary == plane ? 0 : intel_plane->plane + 1,
+ plane->base.id, intel_plane->pipe, intel_plane->plane,
drm_plane_index(plane));
DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = 0x%x",
fb->base.id, fb->width, fb->height, fb->pixel_format);
@@ -13547,13 +13545,11 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
state->scaler_id = -1;
}
primary->pipe = pipe;
- primary->plane = pipe;
+ primary->plane = 0;
primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
primary->check_plane = intel_check_primary_plane;
primary->commit_plane = intel_commit_primary_plane;
primary->disable_plane = intel_disable_primary_plane;
- if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
- primary->plane = !pipe;
if (INTEL_INFO(dev)->gen >= 9) {
intel_primary_formats = skl_primary_formats;
@@ -13699,7 +13695,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
cursor->can_scale = false;
cursor->max_downscale = 1;
cursor->pipe = pipe;
- cursor->plane = pipe;
+ cursor->plane = PLANE_CURSOR;
cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
cursor->check_plane = intel_check_cursor_plane;
cursor->commit_plane = intel_commit_cursor_plane;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 62de97e..9db19f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1131,7 +1131,7 @@ static void vlv_invert_wms(struct intel_crtc *crtc)
wm_state->wm[level].primary;
break;
case DRM_PLANE_TYPE_OVERLAY:
- sprite = plane->plane;
+ sprite = I915_SPRITE_NUM(plane);
wm_state->wm[level].sprite[sprite] = plane->wm.fifo_size -
wm_state->wm[level].sprite[sprite];
break;
@@ -1195,7 +1195,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
wm_state->wm[level].primary = wm;
break;
case DRM_PLANE_TYPE_OVERLAY:
- sprite = plane->plane;
+ sprite = I915_SPRITE_NUM(plane);
wm_state->wm[level].sprite[sprite] = wm;
break;
}
@@ -1221,7 +1221,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
wm_state->wm[level].primary);
break;
case DRM_PLANE_TYPE_OVERLAY:
- sprite = plane->plane;
+ sprite = I915_SPRITE_NUM(plane);
for (level = 0; level < wm_state->num_levels; level++)
wm_state->sr[level].plane =
min(wm_state->sr[level].plane,
@@ -1257,7 +1257,7 @@ static void vlv_pipe_set_fifo_size(struct intel_crtc *crtc)
if (plane->base.type == DRM_PLANE_TYPE_PRIMARY)
sprite0_start = plane->wm.fifo_size;
- else if (plane->plane == 0)
+ else if (I915_SPRITE_NUM(plane) == 0)
sprite1_start = sprite0_start + plane->wm.fifo_size;
else
fifo_size = sprite1_start + plane->wm.fifo_size;
@@ -4076,7 +4076,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, 0);
break;
case DRM_PLANE_TYPE_OVERLAY:
- sprite = plane->plane;
+ sprite = I915_SPRITE_NUM(plane);
plane->wm.fifo_size = vlv_get_fifo_size(dev, plane->pipe, sprite + 1);
break;
}
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 4d27243..11ca40a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -180,7 +180,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
struct intel_plane *intel_plane = to_intel_plane(drm_plane);
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
const int pipe = intel_plane->pipe;
- const int plane = intel_plane->plane + 1;
+ const int plane = I915_UNIVERSAL_NUM(intel_plane);
u32 plane_ctl, stride_div, stride;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
const struct drm_intel_sprite_colorkey *key =
@@ -280,7 +280,7 @@ skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane *intel_plane = to_intel_plane(dplane);
const int pipe = intel_plane->pipe;
- const int plane = intel_plane->plane + 1;
+ const int plane = I915_UNIVERSAL_NUM(intel_plane);
I915_WRITE(PLANE_CTL(pipe, plane), 0);
@@ -294,7 +294,7 @@ static void
chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
{
struct drm_i915_private *dev_priv = intel_plane->base.dev->dev_private;
- int plane = intel_plane->plane;
+ int plane = I915_SPRITE_NUM(intel_plane);
/* Seems RGB data bypasses the CSC always */
if (!format_is_yuv(format))
@@ -342,7 +342,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
struct intel_plane *intel_plane = to_intel_plane(dplane);
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int pipe = intel_plane->pipe;
- int plane = intel_plane->plane;
+ int plane = I915_SPRITE_NUM(intel_plane);
u32 sprctl;
unsigned long sprsurf_offset, linear_offset;
int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
@@ -461,7 +461,7 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_plane *intel_plane = to_intel_plane(dplane);
int pipe = intel_plane->pipe;
- int plane = intel_plane->plane;
+ int plane = I915_SPRITE_NUM(intel_plane);
I915_WRITE(SPCNTR(pipe, plane), 0);
@@ -1121,8 +1121,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
return -ENODEV;
}
+ /* primary = 0, sprites start from 1 */
+ intel_plane->plane = plane + 1;
intel_plane->pipe = pipe;
- intel_plane->plane = plane;
intel_plane->frontbuffer_bit = INTEL_FRONTBUFFER_SPRITE(pipe, plane);
intel_plane->check_plane = intel_check_sprite_plane;
intel_plane->commit_plane = intel_commit_sprite_plane;
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
