[Intel-gfx] [RFC 3/4] drm/i915/gen9: Expose top-most universal plane as cursor

Lyude Paul lyude at redhat.com
Thu Oct 27 22:15:32 UTC 2016


On Wed, 2016-10-26 at 15:51 -0700, Matt Roper wrote:
> Gen9 has a traditional cursor plane that is mutually exclusive with
> the
> system's top-most "universal" plane; it seems likely that two planes
> are
> really a single shared hardware unit with two different register
> interfaces.  Thus far i915 has exposed a cursor plane to userspace
> that's hooked up to the old-style cursor registers; we just pretended
> that the top-most universal plane didn't exist and reported one fewer
> "sprite/overlay" planes for each pipe than the platform technically
> has.
> Let's switch this around so that the cursor exposed to userspace is
> actually wired up to top-most universal plane registers.  We'll
> continue
> to present the same cursor ABI to userspace that we always have, but
> internally we'll just be programming a different set of registers and
> doing so in a way that's more consistent with how all the other gen9
> planes work --- less cursor-specific special cases throughout the
> code.
> 
> Aside from making the code a bit simpler (fewer cursor-specific
> special
> cases), this will also pave the way to eventually exposing the top-
> most
> plane in a more full-featured manner to userspace clients that don't
> need a traditional cursor and wish to opt into having access to an
> additional sprite/overlay-style plane instead.
> 
> It's worth noting that a lot of the special-cased cursor-specific
> code
> was in the gen9 watermark programming.  It's good to simplify that
> code,
> but we should keep an eye out for any unwanted side effects of this
> patch; since sprites/overlays tend to get used less than cursors,
> it's
> possible that this could help us uncover additional underruns that
> nobody had run across yet.  Or it could have the opposite effect and
> eliminate some of the underruns that we haven't been able to get rid
> of
> yet.

Alright, so I had to sit on this patch for a while. I think exposing
this as a normal plane is a good idea: the rest of the world just uses
planes, so we should too. We need to be *really* careful with this
though. Like Paulo said watermarks are still fragile and honestly I
wouldn't be surprised if we found more underruns hiding somewhere.

First off, although this was mentioned in an e-mail down the chain, I'm
pretty sure having a 0 block allocation is definitely not what we want.
The spec itself says a minimum of 8 blocks assuming it's a normal
cursor, so if we're going below that we definitely need to at least
make sure that behavior isn't wrong.

The second thing is that unfortunately this regresses in a rather
strange way. On this X1 yoga w/ a 4K screen (SKL chipset of course), if
I move the cursor in front of a gray background I can see a faintly
visible box starting from the beginning of the cursor and going to the
end of the plane. Reverting the patch fixes the problem.

> 
> Cc: Bob Paauwe <bob.j.paauwe at intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Lyude Paul <lyude at redhat.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |  4 --
>  drivers/gpu/drm/i915/i915_drv.h          | 11 +++-
>  drivers/gpu/drm/i915/intel_device_info.c | 38 +++++++++----
>  drivers/gpu/drm/i915/intel_display.c     | 97 ++++++++++++--------
> ------------
>  drivers/gpu/drm/i915/intel_drv.h         |  7 +++
>  drivers/gpu/drm/i915/intel_pm.c          | 85 ++++----------------
> --------
>  drivers/gpu/drm/i915/intel_sprite.c      |  6 +-
>  7 files changed, 93 insertions(+), 155 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9f5a392..0bba472 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3469,10 +3469,6 @@ static int i915_ddb_info(struct seq_file *m,
> void *unused)
>  				   entry->start, entry->end,
>  				   skl_ddb_entry_size(entry));
>  		}
> -
> -		entry = &ddb->plane[pipe][PLANE_CURSOR];
> -		seq_printf(m, "  %-13s%8u%8u%8u\n", "Cursor", entry-
> >start,
> -			   entry->end, skl_ddb_entry_size(entry));
>  	}
>  
>  	drm_modeset_unlock_all(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 4714051..83aaed2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -178,7 +178,7 @@ enum plane {
>  	PLANE_A = 0,
>  	PLANE_B,
>  	PLANE_C,
> -	PLANE_CURSOR,
> +	PLANE_D,
>  	I915_MAX_PLANES,
>  };
>  #define plane_name(p) ((p) + 'A')
> @@ -316,9 +316,15 @@ struct i915_hotplug {
>  	for ((__p) = 0;						
> 	\
>  	     (__p) < INTEL_INFO(__dev_priv)->num_sprites[(__pipe)] +
> 1;	\
>  	     (__p)++)
> +
> +/*
> + * Only iterate over sprites exposed as sprites; omit sprites that
> + * are being repurposed to simulate a cursor.
> + */
>  #define for_each_sprite(__dev_priv, __p, __s)			
> 	\
>  	for ((__s) = 0;						
> 	\
> -	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];	
> \
> +	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)] -	
> \
> +	             (INTEL_INFO(__dev_priv)->has_real_cursor ? 0 :
> 1);	\
>  	     (__s)++)
>  
>  #define for_each_port_masked(__port, __ports_mask) \
> @@ -687,6 +693,7 @@ struct intel_csr {
>  	func(has_psr); \
>  	func(has_rc6); \
>  	func(has_rc6p); \
> +	func(has_real_cursor); \
>  	func(has_resource_streamer); \
>  	func(has_runtime_pm); \
>  	func(has_snoop); \
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index d6a8f11..a464e0e 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -271,23 +271,39 @@ void intel_device_info_runtime_init(struct
> drm_i915_private *dev_priv)
>  	enum pipe pipe;
>  
>  	/*
> -	 * Skylake and Broxton currently don't expose the topmost
> plane as its
> -	 * use is exclusive with the legacy cursor and we only want
> to expose
> -	 * one of those, not both. Until we can safely expose the
> topmost plane
> -	 * as a DRM_PLANE_TYPE_CURSOR with all the features
> exposed/supported,
> -	 * we don't expose the topmost plane at all to prevent ABI
> breakage
> -	 * down the line.
> +	 * Gen9 platforms have a top-most universal (i.e., sprite)
> plane and a
> +	 * cursor plane that are mutually exclusive.  If we use the
> cursor
> +	 * plane we permanently lose the ability to make use of the
> more
> +	 * full-featured universal plane.  So instead let's use all
> of the
> +	 * universal planes, ignore the cursor plane, but hook the
> top-most
> +	 * universal plane up to the legacy cursor ioctl's and
> expose it to
> +	 * userspace as DRM_PLANE_TYPE_CURSOR.  This won't result in
> any
> +	 * visible behavior change to userspace; we're just
> internally
> +	 * programming a different set of registers.
> +	 *
> +	 * For the purposes of device_info, we're only concerned
> with the
> +	 * number of universal planes we're programming, regardless
> of how they
> +	 * get mapped to userspace interfaces, so we'll report the
> true number of
> +	 * universal planes for gen9.
>  	 */
>  	if (IS_BROXTON(dev_priv)) {
> -		info->num_sprites[PIPE_A] = 2;
> -		info->num_sprites[PIPE_B] = 2;
> -		info->num_sprites[PIPE_C] = 1;
> -	} else if (IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv))
> +		info->has_real_cursor = 0;
> +		info->num_sprites[PIPE_A] = 3;
> +		info->num_sprites[PIPE_B] = 3;
> +		info->num_sprites[PIPE_C] = 2;
> +	} else if (IS_GEN9(dev_priv)) {
> +		info->has_real_cursor = 0;
>  		for_each_pipe(dev_priv, pipe)
>  			info->num_sprites[pipe] = 2;
> -	else
> +	} else if (IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv)) {
> +		info->has_real_cursor = 1;
> +		for_each_pipe(dev_priv, pipe)
> +			info->num_sprites[pipe] = 2;
> +	} else {
> +		info->has_real_cursor = 1;
>  		for_each_pipe(dev_priv, pipe)
>  			info->num_sprites[pipe] = 1;
> +	}
>  
>  	if (i915.disable_display) {
>  		DRM_INFO("Display disabled (module parameter)\n");
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index cb7dd11..9a8c2b1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1232,6 +1232,23 @@ void assert_panel_unlocked(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	     pipe_name(pipe));
>  }
>  
> +/*
> + * Cursor state for platforms that use a universal plane as a
> cursor.
> + * Primary is universal #0, others are universal #1-
> numsprites.  Cursor
> + * will be the final universal plane for the pipe.
> + */
> +static bool
> +universal_cursor_state(struct drm_i915_private *dev_priv,
> +		       enum pipe pipe)
> +{
> +	unsigned int planenum = INTEL_INFO(dev_priv)-
> >num_sprites[pipe];
> +	u32 val;
> +
> +	val = I915_READ(PLANE_CTL(pipe, planenum));
> +
> +	return val & PLANE_CTL_ENABLE;
> +}
> +
>  static void assert_cursor(struct drm_i915_private *dev_priv,
>  			  enum pipe pipe, bool state)
>  {
> @@ -1239,6 +1256,8 @@ static void assert_cursor(struct
> drm_i915_private *dev_priv,
>  
>  	if (IS_845G(dev_priv) || IS_I865G(dev_priv))
>  		cur_state = I915_READ(CURCNTR(PIPE_A)) &
> CURSOR_ENABLE;
> +	else if (!INTEL_INFO(dev_priv)->has_real_cursor)
> +		cur_state = universal_cursor_state(dev_priv, pipe);
>  	else
>  		cur_state = I915_READ(CURCNTR(pipe)) & CURSOR_MODE;
>  
> @@ -10841,15 +10860,16 @@ static void i9xx_update_cursor(struct
> drm_crtc *crtc, u32 base,
>  	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 intel_crtc_state *cstate = to_intel_crtc_state(crtc-
> >state);
> -	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
> -	const struct skl_plane_wm *p_wm =
> -		&cstate->wm.skl.optimal.planes[PLANE_CURSOR];
>  	int pipe = intel_crtc->pipe;
>  	uint32_t cntl = 0;
>  
> -	if (INTEL_GEN(dev_priv) >= 9 && wm->dirty_pipes &
> drm_crtc_mask(crtc))
> -		skl_write_cursor_wm(intel_crtc, p_wm, &wm->ddb);
> +	/*
> +	 * Although gen9 has legacy cursor registers, their use is
> mutually
> +	 * exclusive with the top-most universal plane.  We'll just
> use the
> +	 * universal plane to simulate the legacy cursor interface
> instead,
> +	 * which means we'll never enter here on gen9 platforms.
> +	 */
> +	WARN_ON(IS_GEN9(dev_priv));
>  
>  	if (plane_state && plane_state->base.visible) {
>  		cntl = MCURSOR_GAMMA_ENABLE;
> @@ -13528,56 +13548,6 @@ static void verify_wm_state(struct drm_crtc
> *crtc,
>  				  hw_ddb_entry->start, hw_ddb_entry-
> >end);
>  		}
>  	}
> -
> -	/*
> -	 * cursor
> -	 * If the cursor plane isn't active, we may not have updated
> it's ddb
> -	 * 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) {
> -		hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
> -		sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
> -
> -		/* Watermarks */
> -		for (level = 0; level <= max_level; level++) {
> -			if (skl_wm_level_equals(&hw_plane_wm-
> >wm[level],
> -						&sw_plane_wm-
> >wm[level]))
> -				continue;
> -
> -			DRM_ERROR("mismatch in WM pipe %c cursor
> level %d (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> -				  pipe_name(pipe), level,
> -				  sw_plane_wm->wm[level].plane_en,
> -				  sw_plane_wm-
> >wm[level].plane_res_b,
> -				  sw_plane_wm-
> >wm[level].plane_res_l,
> -				  hw_plane_wm->wm[level].plane_en,
> -				  hw_plane_wm-
> >wm[level].plane_res_b,
> -				  hw_plane_wm-
> >wm[level].plane_res_l);
> -		}
> -
> -		if (!skl_wm_level_equals(&hw_plane_wm->trans_wm,
> -					 &sw_plane_wm->trans_wm)) {
> -			DRM_ERROR("mismatch in trans WM pipe %c
> cursor (expected e=%d b=%u l=%u, got e=%d b=%u l=%u)\n",
> -				  pipe_name(pipe),
> -				  sw_plane_wm->trans_wm.plane_en,
> -				  sw_plane_wm->trans_wm.plane_res_b,
> -				  sw_plane_wm->trans_wm.plane_res_l,
> -				  hw_plane_wm->trans_wm.plane_en,
> -				  hw_plane_wm->trans_wm.plane_res_b,
> -				  hw_plane_wm-
> >trans_wm.plane_res_l);
> -		}
> -
> -		/* DDB */
> -		hw_ddb_entry = &hw_ddb.plane[pipe][PLANE_CURSOR];
> -		sw_ddb_entry = &sw_ddb->plane[pipe][PLANE_CURSOR];
> -
> -		if (!skl_ddb_entry_equal(hw_ddb_entry,
> sw_ddb_entry)) {
> -			DRM_ERROR("mismatch in DDB state pipe %c
> cursor (expected (%u,%u), found (%u,%u))\n",
> -				  pipe_name(pipe),
> -				  sw_ddb_entry->start, sw_ddb_entry-
> >end,
> -				  hw_ddb_entry->start, hw_ddb_entry-
> >end);
> -		}
> -	}
>  }
>  
>  static void
> @@ -15215,11 +15185,18 @@ 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->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 (INTEL_INFO(dev)->has_real_cursor) {
> +		cursor->plane = pipe;  /* unused? */
> +		cursor->check_plane = intel_check_cursor_plane;
> +		cursor->update_plane = intel_update_cursor_plane;
> +		cursor->disable_plane = intel_disable_cursor_plane;
> +	} else {
> +		cursor->plane = INTEL_INFO(dev)->num_sprites[pipe] -
> 1;
> +		cursor->check_plane = intel_check_sprite_plane;
> +		cursor->update_plane = skl_update_plane;
> +		cursor->disable_plane = skl_disable_plane;
> +	}
>  
>  	ret = drm_universal_plane_init(dev, &cursor->base, 0,
>  				       &intel_plane_funcs,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index c31fddd..50874e2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1790,6 +1790,13 @@ int intel_sprite_set_colorkey(struct
> drm_device *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_pipe_update_start(struct intel_crtc *crtc);
>  void intel_pipe_update_end(struct intel_crtc *crtc, struct
> intel_flip_work *work);
> +int intel_check_sprite_plane(struct drm_plane *plane,
> +			     struct intel_crtc_state *crtc_state,
> +			     struct intel_plane_state *state);
> +void skl_update_plane(struct drm_plane *drm_plane,
> +		      const struct intel_crtc_state *crtc_state,
> +		      const struct intel_plane_state *plane_state);
> +void skl_disable_plane(struct drm_plane *dplane, struct drm_crtc
> *crtc);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 6f19e60..e75f6d8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2863,9 +2863,7 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
>  
>  /*
>   * Return the index of a plane in the SKL DDB and wm result
> arrays.  Primary
> - * plane is always in slot 0, cursor is always in slot
> I915_MAX_PLANES-1, and
> - * other universal planes are in indices 1..n.  Note that this may
> leave unused
> - * indices between the top "sprite" plane and the cursor.
> + * plane is always in slot 0 and other universal planes are in
> indices 1..n.
>   */
>  static int
>  skl_wm_plane_id(const struct intel_plane *plane)
> @@ -2874,7 +2872,6 @@ skl_wm_plane_id(const struct intel_plane
> *plane)
>  	case DRM_PLANE_TYPE_PRIMARY:
>  		return 0;
>  	case DRM_PLANE_TYPE_CURSOR:
> -		return PLANE_CURSOR;
>  	case DRM_PLANE_TYPE_OVERLAY:
>  		return plane->plane + 1;
>  	default:
> @@ -3128,14 +3125,6 @@ skl_ddb_get_pipe_allocation_limits(struct
> drm_device *dev,
>  	alloc->end = alloc->start + pipe_size;
>  }
>  
> -static unsigned int skl_cursor_allocation(int num_active)
> -{
> -	if (num_active == 1)
> -		return 32;
> -
> -	return 8;
> -}
> -
>  static void skl_ddb_entry_init_from_hw(struct skl_ddb_entry *entry,
> u32 reg)
>  {
>  	entry->start = reg & 0x3ff;
> @@ -3166,10 +3155,6 @@ void skl_ddb_get_hw_state(struct
> drm_i915_private *dev_priv,
>  						   val);
>  		}
>  
> -		val = I915_READ(CUR_BUF_CFG(pipe));
> -		skl_ddb_entry_init_from_hw(&ddb-
> >plane[pipe][PLANE_CURSOR],
> -					   val);
> -
>  		intel_display_power_put(dev_priv, power_domain);
>  	}
>  }
> @@ -3227,8 +3212,6 @@ skl_plane_relative_data_rate(const struct
> intel_crtc_state *cstate,
>  
>  	if (!intel_pstate->base.visible)
>  		return 0;
> -	if (pstate->plane->type == DRM_PLANE_TYPE_CURSOR)
> -		return 0;
>  	if (y && format != DRM_FORMAT_NV12)
>  		return 0;
>  
> @@ -3386,7 +3369,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
>  	struct drm_plane_state *pstate;
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
> -	uint16_t alloc_size, start, cursor_blocks;
> +	uint16_t alloc_size, start;
>  	uint16_t *minimum = cstate->wm.skl.minimum_blocks;
>  	uint16_t *y_minimum = cstate->wm.skl.minimum_y_blocks;
>  	unsigned int total_data_rate;
> @@ -3412,12 +3395,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
>  		return 0;
>  	}
>  
> -	cursor_blocks = skl_cursor_allocation(num_active);
> -	ddb->plane[pipe][PLANE_CURSOR].start = alloc->end -
> cursor_blocks;
> -	ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
> -
> -	alloc_size -= cursor_blocks;
> -
>  	/* 1. Allocate the mininum required blocks for each active
> plane */
>  	for_each_plane_in_state(state, plane, pstate, i) {
>  		intel_plane = to_intel_plane(plane);
> @@ -3431,17 +3408,12 @@ skl_allocate_pipe_ddb(struct intel_crtc_state
> *cstate,
>  			y_minimum[id] = 0;
>  			continue;
>  		}
> -		if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> -			minimum[id] = 0;
> -			y_minimum[id] = 0;
> -			continue;
> -		}
>  
>  		minimum[id] = skl_ddb_min_alloc(pstate, 0);
>  		y_minimum[id] = skl_ddb_min_alloc(pstate, 1);
>  	}
>  
> -	for (i = 0; i < PLANE_CURSOR; i++) {
> +	for (i = 0; i < I915_MAX_PLANES; i++) {
>  		alloc_size -= minimum[i];
>  		alloc_size -= y_minimum[i];
>  	}
> @@ -3866,26 +3838,6 @@ void skl_write_plane_wm(struct intel_crtc
> *intel_crtc,
>  			    &ddb->y_plane[pipe][plane]);
>  }
>  
> -void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
> -			 const struct skl_plane_wm *wm,
> -			 const struct skl_ddb_allocation *ddb)
> -{
> -	struct drm_crtc *crtc = &intel_crtc->base;
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	int level, max_level = ilk_wm_max_level(dev_priv);
> -	enum pipe pipe = intel_crtc->pipe;
> -
> -	for (level = 0; level <= max_level; level++) {
> -		skl_write_wm_level(dev_priv, CUR_WM(pipe, level),
> -				   &wm->wm[level]);
> -	}
> -	skl_write_wm_level(dev_priv, CUR_WM_TRANS(pipe), &wm-
> >trans_wm);
> -
> -	skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
> -			    &ddb->plane[pipe][PLANE_CURSOR]);
> -}
> -
>  bool skl_wm_level_equals(const struct skl_wm_level *l1,
>  			 const struct skl_wm_level *l2)
>  {
> @@ -4122,19 +4074,11 @@ skl_print_wm_changes(const struct
> drm_atomic_state *state)
>  			if (skl_ddb_entry_equal(old, new))
>  				continue;
>  
> -			if (id != PLANE_CURSOR) {
> -				DRM_DEBUG_ATOMIC("[PLANE:%d:plane
> %d%c] ddb (%d - %d) -> (%d - %d)\n",
> -						 plane->base.id, id
> + 1,
> -						 pipe_name(pipe),
> -						 old->start, old-
> >end,
> -						 new->start, new-
> >end);
> -			} else {
> -				DRM_DEBUG_ATOMIC("[PLANE:%d:cursor
> %c] ddb (%d - %d) -> (%d - %d)\n",
> -						 plane->base.id,
> -						 pipe_name(pipe),
> -						 old->start, old-
> >end,
> -						 new->start, new-
> >end);
> -			}
> +			DRM_DEBUG_ATOMIC("[PLANE:%d:plane %d%c] ddb
> (%d - %d) -> (%d - %d)\n",
> +					 plane->base.id, id + 1,
> +					 pipe_name(pipe),
> +					 old->start, old->end,
> +					 new->start, new->end);
>  		}
>  	}
>  }
> @@ -4235,9 +4179,6 @@ static void skl_update_wm(struct drm_crtc
> *crtc)
>  		for_each_universal_plane(dev_priv, pipe, plane)
>  			skl_write_plane_wm(intel_crtc, &pipe_wm-
> >planes[plane],
>  					   &results->ddb, plane);
> -
> -		skl_write_cursor_wm(intel_crtc, &pipe_wm-
> >planes[PLANE_CURSOR],
> -				    &results->ddb);
>  	}
>  
>  	skl_copy_wm_for_pipe(hw_vals, results, pipe);
> @@ -4350,18 +4291,12 @@ void skl_pipe_wm_get_hw_state(struct drm_crtc
> *crtc,
>  		wm = &out->planes[id];
>  
>  		for (level = 0; level <= max_level; level++) {
> -			if (id != PLANE_CURSOR)
> -				val = I915_READ(PLANE_WM(pipe, id,
> level));
> -			else
> -				val = I915_READ(CUR_WM(pipe,
> level));
> +			val = I915_READ(PLANE_WM(pipe, id, level));
>  
>  			skl_wm_level_from_reg_val(val, &wm-
> >wm[level]);
>  		}
>  
> -		if (id != PLANE_CURSOR)
> -			val = I915_READ(PLANE_WM_TRANS(pipe, id));
> -		else
> -			val = I915_READ(CUR_WM_TRANS(pipe));
> +		val = I915_READ(PLANE_WM_TRANS(pipe, id));
>  
>  		skl_wm_level_from_reg_val(val, &wm->trans_wm);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 43d0350..9e6406a 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -194,7 +194,7 @@ void intel_pipe_update_end(struct intel_crtc
> *crtc, struct intel_flip_work *work
>  	}
>  }
>  
> -static void
> +void
>  skl_update_plane(struct drm_plane *drm_plane,
>  		 const struct intel_crtc_state *crtc_state,
>  		 const struct intel_plane_state *plane_state)
> @@ -285,7 +285,7 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	POSTING_READ(PLANE_SURF(pipe, plane));
>  }
>  
> -static void
> +void
>  skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = dplane->dev;
> @@ -752,7 +752,7 @@ ilk_disable_plane(struct drm_plane *plane, struct
> drm_crtc *crtc)
>  	POSTING_READ(DVSSURF(pipe));
>  }
>  
> -static int
> +int
>  intel_check_sprite_plane(struct drm_plane *plane,
>  			 struct intel_crtc_state *crtc_state,
>  			 struct intel_plane_state *state)
-- 
Cheers,
	Lyude


More information about the Intel-gfx mailing list