All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/11] drm/i915: Use passed plane state for sprite planes.
Date: Thu, 22 Oct 2015 14:58:19 +0200	[thread overview]
Message-ID: <20151022125819.GS16848@phenom.ffwll.local> (raw)
In-Reply-To: <1445514996-18733-2-git-send-email-maarten.lankhorst@linux.intel.com>

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@linux.intel.com>
> ---
>  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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

  reply	other threads:[~2015-10-22 12:58 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-22 11:56 [PATCH 00/11] Kill off intel_crtc->atomic! Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 01/11] drm/i915: Use passed plane state for sprite planes Maarten Lankhorst
2015-10-22 12:58   ` Daniel Vetter [this message]
2015-10-22 13:09     ` Maarten Lankhorst
2015-10-26  9:30     ` [PATCH v2 01/11] drm/i915: Use passed plane state for sprite planes, v2 Maarten Lankhorst
2015-11-17 15:23       ` Daniel Vetter
2015-10-22 11:56 ` [PATCH 02/11] drm/i915: Do not acquire crtc state to check clock during modeset Maarten Lankhorst
2015-10-22 13:08   ` Daniel Vetter
2015-10-22 13:42     ` Maarten Lankhorst
2015-10-22 13:55       ` Daniel Vetter
2015-10-22 13:31   ` Ville Syrjälä
2015-10-27 13:26   ` [PATCH 1/3] drm/i915: Do not acquire crtc state to check clock during modeset, v2 Maarten Lankhorst
2015-10-27 13:26     ` [PATCH 2/3] drm/i915: Handle cdclk limits on broadwell Maarten Lankhorst
2015-10-27 13:26     ` [PATCH 3/3] drm/i915/bxt: Use the bypass frequency if there are no active pipes Maarten Lankhorst
2015-10-27 13:31       ` Ville Syrjälä
2015-10-27 13:29     ` [PATCH 1/3] drm/i915: Do not acquire crtc state to check clock during modeset, v2 Ville Syrjälä
2015-10-27 13:41       ` Maarten Lankhorst
2015-10-27 13:49         ` Ville Syrjälä
2015-10-27 13:51           ` Maarten Lankhorst
2015-10-27 14:27             ` Ville Syrjälä
2015-10-22 11:56 ` [PATCH 03/11] drm/i915: Kill off intel_crtc->atomic.wait_vblank Maarten Lankhorst
2015-10-22 13:09   ` Daniel Vetter
2015-10-22 13:30   ` Ville Syrjälä
2015-10-22 14:50     ` Maarten Lankhorst
2015-10-22 15:15       ` Ville Syrjälä
2015-10-26  8:31         ` Maarten Lankhorst
2015-11-17 15:25           ` Daniel Vetter
2015-10-22 11:56 ` [PATCH 04/11] drm/i915: Update watermark related members in the crtc_state Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 05/11] drm/i915: Remove intel_crtc->atomic.disable_ips Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 06/11] drm/i915: Remove atomic.pre_disable_primary Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 07/11] drm/i915: Remove some post-commit members from intel_crtc->atomic Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 08/11] drm/i915: Nuke fbc " Maarten Lankhorst
2015-10-22 11:56 ` [PATCH 09/11] drm/i915/skl: Prevent unclaimed register writes on skylake Maarten Lankhorst
2015-10-22 11:56   ` Maarten Lankhorst
2015-10-22 13:11   ` [Intel-gfx] " Daniel Vetter
2015-10-23  7:54     ` Jani Nikula
2015-11-02  8:01       ` Jani Nikula
2015-10-22 11:56 ` [PATCH 10/11] drm/i915/skl: Update watermarks before the crtc is disabled Maarten Lankhorst
2015-10-22 11:56   ` Maarten Lankhorst
2015-10-22 13:15   ` [Intel-gfx] " Daniel Vetter
2015-10-22 11:56 ` [PATCH 11/11] drm/i915/skl: Do not allow scaling when " Maarten Lankhorst
2015-10-22 13:17   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151022125819.GS16848@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.