Re: [PATCH 5/7] drm/i915: Use the plane state for cursor updates.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Jan 07, 2016 at 11:54:10AM +0100, Maarten Lankhorst wrote:
> Cursor planes grab the state from plane->state instead of the state
> that was passed. The only updates are atomic now, so use the plane_state
> that's passed in.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Again clashes a bit with my pending stuff. But like said, I can
rebase.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++------------------
>  1 file changed, 46 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7ea49d5e2ce0..f7c293638df4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10076,16 +10076,17 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	return true;
>  }
>  
> -static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
> +static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> +			       const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint32_t cntl = 0, size = 0;
>  
> -	if (on) {
> -		unsigned int width = intel_crtc->base.cursor->state->crtc_w;
> -		unsigned int height = intel_crtc->base.cursor->state->crtc_h;
> +	if (plane_state && plane_state->visible) {
> +		unsigned int width = plane_state->base.crtc_w;
> +		unsigned int height = plane_state->base.crtc_h;
>  		unsigned int stride = roundup_pow_of_two(width) * 4;
>  
>  		switch (stride) {
> @@ -10138,7 +10139,8 @@ static void i845_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  	}
>  }
>  
> -static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
> +static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base,
> +			       const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -10146,9 +10148,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  	int pipe = intel_crtc->pipe;
>  	uint32_t cntl = 0;
>  
> -	if (on) {
> +	if (plane_state && plane_state->visible) {
>  		cntl = MCURSOR_GAMMA_ENABLE;
> -		switch (intel_crtc->base.cursor->state->crtc_w) {
> +		switch (plane_state->base.crtc_w) {
>  			case 64:
>  				cntl |= CURSOR_MODE_64_ARGB_AX;
>  				break;
> @@ -10159,17 +10161,17 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  				cntl |= CURSOR_MODE_256_ARGB_AX;
>  				break;
>  			default:
> -				MISSING_CASE(intel_crtc->base.cursor->state->crtc_w);
> +				MISSING_CASE(plane_state->base.crtc_w);
>  				return;
>  		}
>  		cntl |= pipe << 28; /* Connect to correct pipe */
>  
>  		if (HAS_DDI(dev))
>  			cntl |= CURSOR_PIPE_CSC_ENABLE;
> -	}
>  
> -	if (crtc->cursor->state->rotation == BIT(DRM_ROTATE_180))
> -		cntl |= CURSOR_ROTATE_180;
> +		if (plane_state->base.rotation == BIT(DRM_ROTATE_180))
> +			cntl |= CURSOR_ROTATE_180;
> +	}
>  
>  	if (intel_crtc->cursor_cntl != cntl) {
>  		I915_WRITE(CURCNTR(pipe), cntl);
> @@ -10186,44 +10188,45 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, bool on)
>  
>  /* 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,
> -				     bool on)
> +				     const struct intel_plane_state *plane_state)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int pipe = intel_crtc->pipe;
> -	struct drm_plane_state *cursor_state = crtc->cursor->state;
> -	int x = cursor_state->crtc_x;
> -	int y = cursor_state->crtc_y;
> -	u32 base = 0, pos = 0;
> +	u32 base = intel_crtc->cursor_addr;
> +	u32 pos = 0;
>  
> -	base = intel_crtc->cursor_addr;
> +	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 (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;
> +		if (y < 0) {
> +			pos |= CURSOR_POS_SIGN << CURSOR_Y_SHIFT;
> +			y = -y;
> +		}
> +		pos |= y << CURSOR_Y_SHIFT;
> +
> +		/* ILK+ do this automagically */
> +		if (HAS_GMCH_DISPLAY(dev) &&
> +		    plane_state->base.rotation == BIT(DRM_ROTATE_180)) {
> +			base += (plane_state->base.crtc_h *
> +				 plane_state->base.crtc_w - 1) * 4;
> +		}
>  	}
> -	pos |= y << CURSOR_Y_SHIFT;
>  
>  	I915_WRITE(CURPOS(pipe), pos);
>  
> -	/* ILK+ do this automagically */
> -	if (HAS_GMCH_DISPLAY(dev) &&
> -	    crtc->cursor->state->rotation == BIT(DRM_ROTATE_180)) {
> -		base += (cursor_state->crtc_h *
> -			 cursor_state->crtc_w - 1) * 4;
> -	}
> -
>  	if (IS_845G(dev) || IS_I865G(dev))
> -		i845_update_cursor(crtc, base, on);
> +		i845_update_cursor(crtc, base, plane_state);
>  	else
> -		i9xx_update_cursor(crtc, base, on);
> +		i9xx_update_cursor(crtc, base, plane_state);
>  }
>  
>  static bool cursor_size_ok(struct drm_device *dev,
> @@ -14180,22 +14183,20 @@ intel_disable_cursor_plane(struct drm_plane *plane,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
>  	intel_crtc->cursor_addr = 0;
> -	intel_crtc_update_cursor(crtc, false);
> +	intel_crtc_update_cursor(crtc, NULL);
>  }
>  
>  static void
> -intel_commit_cursor_plane(struct drm_plane *plane,
> -			  struct intel_plane_state *state)
> +intel_update_cursor_plane(struct drm_plane *plane,
> +			  const struct intel_crtc_state *crtc_state,
> +			  const struct intel_plane_state *state)
>  {
> -	struct drm_crtc *crtc = state->base.crtc;
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = plane->dev;
> -	struct intel_crtc *intel_crtc;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
>  	uint32_t addr;
>  
> -	crtc = crtc ? crtc : plane->crtc;
> -	intel_crtc = to_intel_crtc(crtc);
> -
>  	if (!obj)
>  		addr = 0;
>  	else if (!INTEL_INFO(dev)->cursor_needs_physical)
> @@ -14204,8 +14205,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
>  		addr = obj->phys_handle->busaddr;
>  
>  	intel_crtc->cursor_addr = addr;
> -
> -	intel_crtc_update_cursor(crtc, state->visible);
> +	intel_crtc_update_cursor(crtc, state);
>  }
>  
>  static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
> @@ -14231,7 +14231,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
>  	cursor->plane = pipe;
>  	cursor->frontbuffer_bit = INTEL_FRONTBUFFER_CURSOR(pipe);
>  	cursor->check_plane = intel_check_cursor_plane;
> -	cursor->commit_plane = intel_commit_cursor_plane;
> +	cursor->update_plane = intel_update_cursor_plane;
>  	cursor->disable_plane = intel_disable_cursor_plane;
>  
>  	drm_universal_plane_init(dev, &cursor->base, 0,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx