Re: [PATCH 01/11] drm/i915: Use passed plane state for sprite planes.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 22, 2015 at 01:56:26PM +0200, Maarten Lankhorst wrote:
> Don't use plane->state directly, use the pointer from commit_plane.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    |  8 ++---
>  drivers/gpu/drm/i915/intel_sprite.c | 67 +++++++++++++++++++++++--------------
>  2 files changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4e35557bbd41..51722e657b91 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -631,16 +631,16 @@ struct intel_plane {
>  	/*
>  	 * NOTE: Do not place new plane state fields here (e.g., when adding
>  	 * new plane properties).  New runtime state should now be placed in
> -	 * the intel_plane_state structure and accessed via drm_plane->state.
> +	 * the intel_plane_state structure and accessed via plane_state.
>  	 */
>  
>  	void (*update_plane)(struct drm_plane *plane,
> -			     struct drm_crtc *crtc,
> -			     struct drm_framebuffer *fb,
> +			     struct intel_plane_state *plane_state,
>  			     int crtc_x, int crtc_y,
>  			     unsigned int crtc_w, unsigned int crtc_h,
>  			     uint32_t x, uint32_t y,
> -			     uint32_t src_w, uint32_t src_h);
> +			     uint32_t src_w, uint32_t src_h,
> +			     struct intel_scaler *scaler);

This is an ugly hack imo. Better to add the scaler pointer to the plane
state, set it in compute_config once we're done and not pass it around
like this. Or we also pass around a pointer to the right crtc_state.

Also, if we pass around plane_state I think all the crtc_x/y/w/h and x/y
and src_x/y/w/h should disappear, for a really clean vfunc interface.
-Daniel

>  	void (*disable_plane)(struct drm_plane *plane,
>  			      struct drm_crtc *crtc);
>  	int (*check_plane)(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 2551335ada04..599d001b1415 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -178,16 +178,19 @@ void intel_pipe_update_end(struct intel_crtc *crtc)
>  }
>  
>  static void
> -skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> +skl_update_plane(struct drm_plane *drm_plane,
> +		 struct intel_plane_state *plane_state,
>  		 int crtc_x, int crtc_y,
>  		 unsigned int crtc_w, unsigned int crtc_h,
>  		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +		 uint32_t src_w, uint32_t src_h,
> +		 struct intel_scaler *scaler)
>  {
>  	struct drm_device *dev = drm_plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_crtc *crtc = plane_state->base.crtc;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	const int pipe = intel_plane->pipe;
>  	const int plane = intel_plane->plane + 1;
> @@ -199,8 +202,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	u32 tile_height, plane_offset, plane_size;
>  	unsigned int rotation;
>  	int x_offset, y_offset;
> -	struct intel_crtc_state *crtc_state = to_intel_crtc(crtc)->config;
> -	int scaler_id;
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -219,8 +220,6 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
>  					       fb->pixel_format);
>  
> -	scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
> -
>  	/* Sizes are 0 based */
>  	src_w--;
>  	src_h--;
> @@ -261,13 +260,13 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  	I915_WRITE(PLANE_SIZE(pipe, plane), plane_size);
>  
>  	/* program plane scaler */
> -	if (scaler_id >= 0) {
> +	if (scaler) {
>  		uint32_t ps_ctrl = 0;
> +		int scaler_id = plane_state->scaler_id;
>  
>  		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
>  			PS_PLANE_SEL(plane));
> -		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) |
> -			crtc_state->scaler_state.scalers[scaler_id].mode;
> +		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->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), (crtc_x << 16) | crtc_y);
> @@ -341,16 +340,18 @@ chv_update_csc(struct intel_plane *intel_plane, uint32_t format)
>  }
>  
>  static void
> -vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> +vlv_update_plane(struct drm_plane *dplane,
> +		 struct intel_plane_state *plane_state,
>  		 int crtc_x, int crtc_y,
>  		 unsigned int crtc_w, unsigned int crtc_h,
>  		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +		 uint32_t src_w, uint32_t src_h,
> +		 struct intel_scaler *scaler)
>  {
>  	struct drm_device *dev = dplane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(dplane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int pipe = intel_plane->pipe;
>  	int plane = intel_plane->plane;
> @@ -481,16 +482,19 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>  }
>  
>  static void
> -ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> +ivb_update_plane(struct drm_plane *plane,
> +		 struct intel_plane_state *plane_state,
>  		 int crtc_x, int crtc_y,
>  		 unsigned int crtc_w, unsigned int crtc_h,
>  		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +		 uint32_t src_w, uint32_t src_h,
> +		 struct intel_scaler *scaler)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_crtc *crtc = plane_state->base.crtc;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	enum pipe pipe = intel_plane->pipe;
>  	u32 sprctl, sprscale = 0;
> @@ -623,16 +627,19 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>  }
>  
>  static void
> -ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -		 struct drm_framebuffer *fb,
> +ilk_update_plane(struct drm_plane *plane,
> +		 struct intel_plane_state *plane_state,
>  		 int crtc_x, int crtc_y,
>  		 unsigned int crtc_w, unsigned int crtc_h,
>  		 uint32_t x, uint32_t y,
> -		 uint32_t src_w, uint32_t src_h)
> +		 uint32_t src_w, uint32_t src_h,
> +		 struct intel_scaler *scaler)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct drm_crtc *crtc = plane_state->base.crtc;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int pipe = intel_plane->pipe;
>  	unsigned long dvssurf_offset, linear_offset;
> @@ -932,23 +939,31 @@ static void
>  intel_commit_sprite_plane(struct drm_plane *plane,
>  			  struct intel_plane_state *state)
>  {
> -	struct drm_crtc *crtc = state->base.crtc;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> -	struct drm_framebuffer *fb = state->base.fb;
> -
> -	crtc = crtc ? crtc : plane->crtc;
>  
>  	if (state->visible) {
> -		intel_plane->update_plane(plane, crtc, fb,
> +		struct intel_scaler *scaler = NULL;
> +
> +		if (state->scaler_id >= 0) {
> +			struct intel_crtc_state *crtc_state =
> +				to_intel_crtc(state->base.crtc)->config;
> +
> +			scaler = &crtc_state->scaler_state.scalers[state->scaler_id];
> +		}
> +
> +		intel_plane->update_plane(plane, state,
>  					  state->dst.x1, state->dst.y1,
>  					  drm_rect_width(&state->dst),
>  					  drm_rect_height(&state->dst),
>  					  state->src.x1 >> 16,
>  					  state->src.y1 >> 16,
>  					  drm_rect_width(&state->src) >> 16,
> -					  drm_rect_height(&state->src) >> 16);
> +					  drm_rect_height(&state->src) >> 16,
> +					  scaler);
>  	} else {
> -		intel_plane->disable_plane(plane, crtc);
> +		struct drm_crtc *crtc = state->base.crtc;
> +
> +		intel_plane->disable_plane(plane, crtc ?: plane->crtc);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux