Re: [PATCH 5/9] drm/i915: Use enum plane_id in SKL plane code

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Ter, 2016-11-08 às 16:47 +0200, ville.syrjala@linux.intel.com
escreveu:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Replace the intel_plane->plane and hardcoded 0 usage in the SKL plane
> code with intel_plane->id.
> 
> This should make the SKL "primary" and "sprite" code virtually
> identical, so the next logical step would likely be dropping one
> of the copies.

Something something 80 columns something something. Also, this will
need a rebase, but the idea is sane, so with a bug-free rebase:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>.

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++++------------
>  drivers/gpu/drm/i915/intel_sprite.c  | 50 ++++++++++++++++++------
> ------------
>  2 files changed, 43 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index e3ed5d1fcf0d..95644c8cc568 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3384,9 +3384,10 @@ static void
> skylake_update_primary_plane(struct drm_plane *plane,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_crtc *intel_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;
>  	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
>  	const struct skl_plane_wm *p_wm =
> -		&crtc_state->wm.skl.optimal.planes[0];
> +		&crtc_state->wm.skl.optimal.planes[plane_id];
>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl;
>  	unsigned int rotation = plane_state->base.rotation;
> @@ -3423,32 +3424,32 @@ static void
> skylake_update_primary_plane(struct drm_plane *plane,
>  	intel_crtc->adjusted_y = src_y;
>  
>  	if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base))
> -		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb, 0);
> +		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb,
> plane_id);
>  
> -	I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
> -	I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x);
> -	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> -	I915_WRITE(PLANE_SIZE(pipe, 0), (src_h << 16) | src_w);
> +	I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
> +	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (src_y << 16) |
> src_x);
> +	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
> +	I915_WRITE(PLANE_SIZE(pipe, plane_id), (src_h << 16) |
> src_w);
>  
>  	if (scaler_id >= 0) {
>  		uint32_t ps_ctrl = 0;
>  
>  		WARN_ON(!dst_w || !dst_h);
> -		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(0) |
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
>  			crtc_state-
> >scaler_state.scalers[scaler_id].mode;
>  		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
>  		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (dst_x
> << 16) | dst_y);
>  		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id), (dst_w <<
> 16) | dst_h);
> -		I915_WRITE(PLANE_POS(pipe, 0), 0);
> +		I915_WRITE(PLANE_POS(pipe, plane_id), 0);
>  	} else {
> -		I915_WRITE(PLANE_POS(pipe, 0), (dst_y << 16) |
> dst_x);
> +		I915_WRITE(PLANE_POS(pipe, plane_id), (dst_y << 16)
> | dst_x);
>  	}
>  
> -	I915_WRITE(PLANE_SURF(pipe, 0),
> +	I915_WRITE(PLANE_SURF(pipe, plane_id),
>  		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
>  
> -	POSTING_READ(PLANE_SURF(pipe, 0));
> +	POSTING_READ(PLANE_SURF(pipe, plane_id));
>  }
>  
>  static void skylake_disable_primary_plane(struct drm_plane *primary,
> @@ -3458,7 +3459,8 @@ static void
> skylake_disable_primary_plane(struct drm_plane *primary,
>  	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_plane_wm *p_wm = &cstate-
> >wm.skl.optimal.planes[0];
> +	enum plane_id plane_id = to_intel_plane(primary)->id;
> +	const struct skl_plane_wm *p_wm = &cstate-
> >wm.skl.optimal.planes[plane_id];
>  	int pipe = intel_crtc->pipe;
>  
>  	/*
> @@ -3467,11 +3469,11 @@ static void
> skylake_disable_primary_plane(struct drm_plane *primary,
>  	 */
>  	if (!crtc->primary->state->visible)
>  		skl_write_plane_wm(intel_crtc, p_wm,
> -				   &dev_priv->wm.skl_results.ddb,
> 0);
> +				   &dev_priv->wm.skl_results.ddb,
> plane_id);
>  
> -	I915_WRITE(PLANE_CTL(pipe, 0), 0);
> -	I915_WRITE(PLANE_SURF(pipe, 0), 0);
> -	POSTING_READ(PLANE_SURF(pipe, 0));
> +	I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
> +	I915_WRITE(PLANE_SURF(pipe, plane_id), 0);
> +	POSTING_READ(PLANE_SURF(pipe, plane_id));
>  }
>  
>  /* Assume fb object is pinned & idle & fenced and just update base
> pointers */
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 4b44863a07c2..91d47d19f4a9 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,13 +203,13 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
>  	struct drm_framebuffer *fb = plane_state->base.fb;
> +	enum plane_id plane_id = intel_plane->id;
>  	const struct skl_wm_values *wm = &dev_priv->wm.skl_results;
>  	struct drm_crtc *crtc = crtc_state->base.crtc;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	const int pipe = intel_plane->pipe;
> -	const int plane = intel_plane->plane + 1;
> +	enum pipe pipe = intel_plane->pipe;
>  	const struct skl_plane_wm *p_wm =
> -		&crtc_state->wm.skl.optimal.planes[plane];
> +		&crtc_state->wm.skl.optimal.planes[plane_id];
>  	u32 plane_ctl;
>  	const struct drm_intel_sprite_colorkey *key = &plane_state-
> >ckey;
>  	u32 surf_addr = plane_state->main.offset;
> @@ -234,12 +234,12 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	plane_ctl |= skl_plane_ctl_rotation(rotation);
>  
>  	if (wm->dirty_pipes & drm_crtc_mask(crtc))
> -		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb,
> plane);
> +		skl_write_plane_wm(intel_crtc, p_wm, &wm->ddb,
> plane_id);
>  
>  	if (key->flags) {
> -		I915_WRITE(PLANE_KEYVAL(pipe, plane), key-
> >min_value);
> -		I915_WRITE(PLANE_KEYMAX(pipe, plane), key-
> >max_value);
> -		I915_WRITE(PLANE_KEYMSK(pipe, plane), key-
> >channel_mask);
> +		I915_WRITE(PLANE_KEYVAL(pipe, plane_id), key-
> >min_value);
> +		I915_WRITE(PLANE_KEYMAX(pipe, plane_id), key-
> >max_value);
> +		I915_WRITE(PLANE_KEYMSK(pipe, plane_id), key-
> >channel_mask);
>  	}
>  
>  	if (key->flags & I915_SET_COLORKEY_DESTINATION)
> @@ -253,36 +253,36 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	I915_WRITE(PLANE_OFFSET(pipe, plane), (y << 16) | x);
> -	I915_WRITE(PLANE_STRIDE(pipe, plane), stride);
> -	I915_WRITE(PLANE_SIZE(pipe, plane), (src_h << 16) | src_w);
> +	I915_WRITE(PLANE_OFFSET(pipe, plane_id), (y << 16) | x);
> +	I915_WRITE(PLANE_STRIDE(pipe, plane_id), stride);
> +	I915_WRITE(PLANE_SIZE(pipe, plane_id), (src_h << 16) |
> src_w);
>  
>  	/* program plane scaler */
>  	if (plane_state->scaler_id >= 0) {
>  		int scaler_id = plane_state->scaler_id;
>  		const struct intel_scaler *scaler;
>  
> -		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) =
> 0x%x\n", plane,
> -			PS_PLANE_SEL(plane));
> +		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) =
> 0x%x\n", plane_id,
> +			      PS_PLANE_SEL(plane_id));
>  
>  		scaler = &crtc_state-
> >scaler_state.scalers[scaler_id];
>  
>  		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id),
> -			   PS_SCALER_EN | PS_PLANE_SEL(plane) |
> scaler->mode);
> +			   PS_SCALER_EN | PS_PLANE_SEL(plane_id) |
> scaler->mode);
>  		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x
> << 16) | crtc_y);
>  		I915_WRITE(SKL_PS_WIN_SZ(pipe, scaler_id),
>  			((crtc_w + 1) << 16)|(crtc_h + 1));
>  
> -		I915_WRITE(PLANE_POS(pipe, plane), 0);
> +		I915_WRITE(PLANE_POS(pipe, plane_id), 0);
>  	} else {
> -		I915_WRITE(PLANE_POS(pipe, plane), (crtc_y << 16) |
> crtc_x);
> +		I915_WRITE(PLANE_POS(pipe, plane_id), (crtc_y << 16)
> | crtc_x);
>  	}
>  
> -	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
> -	I915_WRITE(PLANE_SURF(pipe, plane),
> +	I915_WRITE(PLANE_CTL(pipe, plane_id), plane_ctl);
> +	I915_WRITE(PLANE_SURF(pipe, plane_id),
>  		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
> -	POSTING_READ(PLANE_SURF(pipe, plane));
> +	POSTING_READ(PLANE_SURF(pipe, plane_id));
>  }
>  
>  static void
> @@ -292,8 +292,8 @@ skl_disable_plane(struct drm_plane *dplane,
> struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane *intel_plane = to_intel_plane(dplane);
>  	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc-
> >state);
> -	const int pipe = intel_plane->pipe;
> -	const int plane = intel_plane->plane + 1;
> +	enum pipe pipe = intel_plane->pipe;
> +	enum plane_id plane_id = intel_plane->id;
>  
>  	/*
>  	 * We only populate skl_results on watermark updates, and if
> the
> @@ -301,13 +301,13 @@ skl_disable_plane(struct drm_plane *dplane,
> struct drm_crtc *crtc)
>  	 */
>  	if (!dplane->state->visible)
>  		skl_write_plane_wm(to_intel_crtc(crtc),
> -				   &cstate-
> >wm.skl.optimal.planes[plane],
> -				   &dev_priv->wm.skl_results.ddb,
> plane);
> +				   &cstate-
> >wm.skl.optimal.planes[plane_id],
> +				   &dev_priv->wm.skl_results.ddb,
> plane_id);
>  
> -	I915_WRITE(PLANE_CTL(pipe, plane), 0);
> +	I915_WRITE(PLANE_CTL(pipe, plane_id), 0);
>  
> -	I915_WRITE(PLANE_SURF(pipe, plane), 0);
> -	POSTING_READ(PLANE_SURF(pipe, plane));
> +	I915_WRITE(PLANE_SURF(pipe, plane_id), 0);
> +	POSTING_READ(PLANE_SURF(pipe, plane_id));
>  }
>  
>  static void
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx