diff mbox series

[04/13] drm/i915: Rename the plane_state->main/aux to plane_state->color_plane[]

Message ID 20180907152413.15761-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Display gtt remapping prep stuff | expand

Commit Message

Ville Syrjälä Sept. 7, 2018, 3:24 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make the main/aux surface stuff a bit more generic by using an array
of structures. This will allow us to deal with both the main and aux
surfaces with common code.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 56 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  6 +---
 drivers/gpu/drm/i915/intel_fbc.c     |  4 +--
 drivers/gpu/drm/i915/intel_sprite.c  | 29 ++++++++++---------
 4 files changed, 46 insertions(+), 49 deletions(-)

Comments

Chris Wilson Sept. 7, 2018, 3:30 p.m. UTC | #1 
Quoting Ville Syrjala (2018-09-07 16:24:04)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8162025114f5..9e16bdcffc84 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -504,11 +504,7 @@ struct intel_plane_state {
>         struct {
>                 u32 offset;
>                 int x, y;
> -       } main;
> -       struct {
> -               u32 offset;
> -               int x, y;
> -       } aux;
> +       } color_plane[2];

Is (main, aux) significant to keep around as an enum?
-Chris
Ville Syrjälä Sept. 7, 2018, 3:54 p.m. UTC | #2 
On Fri, Sep 07, 2018 at 04:30:36PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2018-09-07 16:24:04)
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 8162025114f5..9e16bdcffc84 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -504,11 +504,7 @@ struct intel_plane_state {
> >         struct {
> >                 u32 offset;
> >                 int x, y;
> > -       } main;
> > -       struct {
> > -               u32 offset;
> > -               int x, y;
> > -       } aux;
> > +       } color_plane[2];
> 
> Is (main, aux) significant to keep around as an enum?

The main/aux names are specific to our hardware design whereas we
often just index this stuff using the color plane index as used
by drm where the main/aux terminology doesn't apply. So we would
have some code indexing this with just a raw int and some using
the enum. Not sure how confusing that would end up being.

Also with icl nv12 support we'll actually just end up using two
hardware planes where neither one will get its aux surface registers
programmed. So if we still stick to a single shared plane state for
both hardware planes the main/aux terminology no longer really applies.
But the details of this are still up in the air so not sure how it'll
end up looking.

So I guess I'd probably just go with no enum and make it clear that
this stuff refers to the drm color plane index instead of the hardware
main and aux surfaces, even though currently they do end up being the
same thing.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 417d5d24cc8d..98c30a40078e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2951,9 +2951,9 @@  static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	int hsub = fb->format->hsub;
 	int vsub = fb->format->vsub;
-	int aux_x = plane_state->aux.x;
-	int aux_y = plane_state->aux.y;
-	u32 aux_offset = plane_state->aux.offset;
+	int aux_x = plane_state->color_plane[1].x;
+	int aux_y = plane_state->color_plane[1].y;
+	u32 aux_offset = plane_state->color_plane[1].offset;
 	u32 alignment = intel_surf_alignment(fb, 1);
 
 	while (aux_offset >= main_offset && aux_y <= main_y) {
@@ -2976,9 +2976,9 @@  static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
 	if (aux_x != main_x || aux_y != main_y)
 		return false;
 
-	plane_state->aux.offset = aux_offset;
-	plane_state->aux.x = aux_x;
-	plane_state->aux.y = aux_y;
+	plane_state->color_plane[1].offset = aux_offset;
+	plane_state->color_plane[1].x = aux_x;
+	plane_state->color_plane[1].y = aux_y;
 
 	return true;
 }
@@ -2999,7 +2999,7 @@  static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
 	int pipe_src_w = crtc_state->pipe_src_w;
 	int max_width = skl_max_plane_width(fb, 0, rotation);
 	int max_height = 4096;
-	u32 alignment, offset, aux_offset = plane_state->aux.offset;
+	u32 alignment, offset, aux_offset = plane_state->color_plane[1].offset;
 
 	if (w > max_width || h > max_height) {
 		DRM_DEBUG_KMS("requested Y/RGB source size %dx%d too big (limit %dx%d)\n",
@@ -3071,15 +3071,15 @@  static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
 								   offset, offset - alignment);
 		}
 
-		if (x != plane_state->aux.x || y != plane_state->aux.y) {
+		if (x != plane_state->color_plane[1].x || y != plane_state->color_plane[1].y) {
 			DRM_DEBUG_KMS("Unable to find suitable display surface offset due to CCS\n");
 			return -EINVAL;
 		}
 	}
 
-	plane_state->main.offset = offset;
-	plane_state->main.x = x;
-	plane_state->main.y = y;
+	plane_state->color_plane[0].offset = offset;
+	plane_state->color_plane[0].x = x;
+	plane_state->color_plane[0].y = y;
 
 	return 0;
 }
@@ -3129,9 +3129,9 @@  static int skl_check_nv12_aux_surface(struct intel_plane_state *plane_state)
 		return -EINVAL;
 	}
 
-	plane_state->aux.offset = offset;
-	plane_state->aux.x = x;
-	plane_state->aux.y = y;
+	plane_state->color_plane[1].offset = offset;
+	plane_state->color_plane[1].x = x;
+	plane_state->color_plane[1].y = y;
 
 	return 0;
 }
@@ -3156,9 +3156,9 @@  static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
 	intel_add_fb_offsets(&x, &y, plane_state, 1);
 	offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 1);
 
-	plane_state->aux.offset = offset;
-	plane_state->aux.x = x * hsub + src_x % hsub;
-	plane_state->aux.y = y * vsub + src_y % vsub;
+	plane_state->color_plane[1].offset = offset;
+	plane_state->color_plane[1].x = x * hsub + src_x % hsub;
+	plane_state->color_plane[1].y = y * vsub + src_y % vsub;
 
 	return 0;
 }
@@ -3201,9 +3201,9 @@  int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
 		if (ret)
 			return ret;
 	} else {
-		plane_state->aux.offset = ~0xfff;
-		plane_state->aux.x = 0;
-		plane_state->aux.y = 0;
+		plane_state->color_plane[1].offset = ~0xfff;
+		plane_state->color_plane[1].x = 0;
+		plane_state->color_plane[1].y = 0;
 	}
 
 	ret = skl_check_main_surface(crtc_state, plane_state);
@@ -3332,9 +3332,9 @@  int i9xx_check_plane_surface(struct intel_plane_state *plane_state)
 		}
 	}
 
-	plane_state->main.offset = offset;
-	plane_state->main.x = src_x;
-	plane_state->main.y = src_y;
+	plane_state->color_plane[0].offset = offset;
+	plane_state->color_plane[0].x = src_x;
+	plane_state->color_plane[0].y = src_y;
 
 	return 0;
 }
@@ -3349,15 +3349,15 @@  static void i9xx_update_plane(struct intel_plane *plane,
 	u32 linear_offset;
 	u32 dspcntr = plane_state->ctl;
 	i915_reg_t reg = DSPCNTR(i9xx_plane);
-	int x = plane_state->main.x;
-	int y = plane_state->main.y;
+	int x = plane_state->color_plane[0].x;
+	int y = plane_state->color_plane[0].y;
 	unsigned long irqflags;
 	u32 dspaddr_offset;
 
 	linear_offset = intel_fb_xy_to_linear(x, y, plane_state, 0);
 
 	if (INTEL_GEN(dev_priv) >= 4)
-		dspaddr_offset = plane_state->main.offset;
+		dspaddr_offset = plane_state->color_plane[0].offset;
 	else
 		dspaddr_offset = linear_offset;
 
@@ -9622,7 +9622,7 @@  static u32 intel_cursor_base(const struct intel_plane_state *plane_state)
 	else
 		base = intel_plane_ggtt_offset(plane_state);
 
-	base += plane_state->main.offset;
+	base += plane_state->color_plane[0].offset;
 
 	/* ILK+ do this automagically */
 	if (HAS_GMCH_DISPLAY(dev_priv) &&
@@ -9701,7 +9701,7 @@  static int intel_check_cursor(struct intel_crtc_state *crtc_state,
 		return -EINVAL;
 	}
 
-	plane_state->main.offset = offset;
+	plane_state->color_plane[0].offset = offset;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8162025114f5..9e16bdcffc84 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -504,11 +504,7 @@  struct intel_plane_state {
 	struct {
 		u32 offset;
 		int x, y;
-	} main;
-	struct {
-		u32 offset;
-		int x, y;
-	} aux;
+	} color_plane[2];
 
 	/* plane control register */
 	u32 ctl;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 01d1d2088f04..74d425c700ef 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -670,8 +670,8 @@  static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	cache->plane.src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	cache->plane.src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	cache->plane.visible = plane_state->base.visible;
-	cache->plane.adjusted_x = plane_state->main.x;
-	cache->plane.adjusted_y = plane_state->main.y;
+	cache->plane.adjusted_x = plane_state->color_plane[0].x;
+	cache->plane.adjusted_y = plane_state->color_plane[0].y;
 	cache->plane.y = plane_state->base.src.y1 >> 16;
 
 	if (!cache->plane.visible)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9c6278792755..df0cf3d2ea98 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -258,7 +258,7 @@  skl_update_plane(struct intel_plane *plane,
 	enum pipe pipe = plane->pipe;
 	u32 plane_ctl = plane_state->ctl;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
-	u32 surf_addr = plane_state->main.offset;
+	u32 surf_addr = plane_state->color_plane[0].offset;
 	unsigned int rotation = plane_state->base.rotation;
 	u32 stride = skl_plane_stride(fb, 0, rotation);
 	u32 aux_stride = skl_plane_stride(fb, 1, rotation);
@@ -266,8 +266,8 @@  skl_update_plane(struct intel_plane *plane,
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->main.x;
-	uint32_t y = plane_state->main.y;
+	uint32_t x = plane_state->color_plane[0].x;
+	uint32_t y = plane_state->color_plane[0].y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
@@ -294,9 +294,10 @@  skl_update_plane(struct intel_plane *plane,
 	I915_WRITE_FW(PLANE_STRIDE(pipe, plane_id), stride);
 	I915_WRITE_FW(PLANE_SIZE(pipe, plane_id), (src_h << 16) | src_w);
 	I915_WRITE_FW(PLANE_AUX_DIST(pipe, plane_id),
-		      (plane_state->aux.offset - surf_addr) | aux_stride);
+		      (plane_state->color_plane[1].offset - surf_addr) | aux_stride);
 	I915_WRITE_FW(PLANE_AUX_OFFSET(pipe, plane_id),
-		      (plane_state->aux.y << 16) | plane_state->aux.x);
+		      (plane_state->color_plane[1].y << 16) |
+		      plane_state->color_plane[1].x);
 
 	/* program plane scaler */
 	if (plane_state->scaler_id >= 0) {
@@ -562,15 +563,15 @@  vlv_update_plane(struct intel_plane *plane,
 	enum pipe pipe = plane->pipe;
 	enum plane_id plane_id = plane->id;
 	u32 sprctl = plane_state->ctl;
-	u32 sprsurf_offset = plane_state->main.offset;
+	u32 sprsurf_offset = plane_state->color_plane[0].offset;
 	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->main.x;
-	uint32_t y = plane_state->main.y;
+	uint32_t x = plane_state->color_plane[0].x;
+	uint32_t y = plane_state->color_plane[0].y;
 	unsigned long irqflags;
 
 	/* Sizes are 0 based */
@@ -721,15 +722,15 @@  ivb_update_plane(struct intel_plane *plane,
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	enum pipe pipe = plane->pipe;
 	u32 sprctl = plane_state->ctl, sprscale = 0;
-	u32 sprsurf_offset = plane_state->main.offset;
+	u32 sprsurf_offset = plane_state->color_plane[0].offset;
 	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->main.x;
-	uint32_t y = plane_state->main.y;
+	uint32_t x = plane_state->color_plane[0].x;
+	uint32_t y = plane_state->color_plane[0].y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;
@@ -893,15 +894,15 @@  g4x_update_plane(struct intel_plane *plane,
 	const struct drm_framebuffer *fb = plane_state->base.fb;
 	enum pipe pipe = plane->pipe;
 	u32 dvscntr = plane_state->ctl, dvsscale = 0;
-	u32 dvssurf_offset = plane_state->main.offset;
+	u32 dvssurf_offset = plane_state->color_plane[0].offset;
 	u32 linear_offset;
 	const struct drm_intel_sprite_colorkey *key = &plane_state->ckey;
 	int crtc_x = plane_state->base.dst.x1;
 	int crtc_y = plane_state->base.dst.y1;
 	uint32_t crtc_w = drm_rect_width(&plane_state->base.dst);
 	uint32_t crtc_h = drm_rect_height(&plane_state->base.dst);
-	uint32_t x = plane_state->main.x;
-	uint32_t y = plane_state->main.y;
+	uint32_t x = plane_state->color_plane[0].x;
+	uint32_t y = plane_state->color_plane[0].y;
 	uint32_t src_w = drm_rect_width(&plane_state->base.src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->base.src) >> 16;
 	unsigned long irqflags;