public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/5] drm: Pass in new and old plane state to prepare_fb and cleanup_fb
Date: Mon, 2 Mar 2015 16:51:04 +0100	[thread overview]
Message-ID: <20150302155104.GA24485@phenom.ffwll.local> (raw)
In-Reply-To: <1425307432-4955-2-git-send-email-tvrtko.ursulin@linux.intel.com>

On Mon, Mar 02, 2015 at 02:43:48PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Use cases like rotation require these hooks to have some context so they
> know how to prepare and cleanup the frame buffer correctly.
> 
> For i915 specifically, object backing pages need to be mapped differently
> for different rotation modes and the driver needs to know which mapping to
> instantiate and which to tear down when transitioning between them.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org

Since I'm still somewhat afraid that drivers will abuse this creatively
and do some state computations in prepare_fb (which they absolutely may
not) I think it would be good to make this new parameter const. I plan to
follow up with other patches to make state objects const in the commit
phase.
-Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c       | 13 ++++++++-----
>  drivers/gpu/drm/drm_plane_helper.c        |  5 +++--
>  drivers/gpu/drm/i915/intel_display.c      |  6 ++++--
>  drivers/gpu/drm/i915/intel_drv.h          |  6 ++++--
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c |  6 ++++--
>  drivers/gpu/drm/tegra/dc.c                |  6 ++++--
>  include/drm/drm_plane_helper.h            |  6 ++++--
>  7 files changed, 31 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 3ce57f4..a745881 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1116,6 +1116,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  	for (i = 0; i < nplanes; i++) {
>  		struct drm_plane_helper_funcs *funcs;
>  		struct drm_plane *plane = state->planes[i];
> +		struct drm_plane_state *plane_state = state->plane_states[i];
>  		struct drm_framebuffer *fb;
>  
>  		if (!plane)
> @@ -1123,10 +1124,10 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
>  
>  		funcs = plane->helper_private;
>  
> -		fb = state->plane_states[i]->fb;
> +		fb = plane_state->fb;
>  
>  		if (fb && funcs->prepare_fb) {
> -			ret = funcs->prepare_fb(plane, fb);
> +			ret = funcs->prepare_fb(plane, fb, plane_state);
>  			if (ret)
>  				goto fail;
>  		}
> @@ -1138,6 +1139,7 @@ fail:
>  	for (i--; i >= 0; i--) {
>  		struct drm_plane_helper_funcs *funcs;
>  		struct drm_plane *plane = state->planes[i];
> +		struct drm_plane_state *plane_state = state->plane_states[i];
>  		struct drm_framebuffer *fb;
>  
>  		if (!plane)
> @@ -1148,7 +1150,7 @@ fail:
>  		fb = state->plane_states[i]->fb;
>  
>  		if (fb && funcs->cleanup_fb)
> -			funcs->cleanup_fb(plane, fb);
> +			funcs->cleanup_fb(plane, fb, plane_state);
>  
>  	}
>  
> @@ -1254,6 +1256,7 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  	for (i = 0; i < nplanes; i++) {
>  		struct drm_plane_helper_funcs *funcs;
>  		struct drm_plane *plane = old_state->planes[i];
> +		struct drm_plane_state *plane_state = old_state->plane_states[i];
>  		struct drm_framebuffer *old_fb;
>  
>  		if (!plane)
> @@ -1261,10 +1264,10 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
>  
>  		funcs = plane->helper_private;
>  
> -		old_fb = old_state->plane_states[i]->fb;
> +		old_fb = plane_state->fb;
>  
>  		if (old_fb && funcs->cleanup_fb)
> -			funcs->cleanup_fb(plane, old_fb);
> +			funcs->cleanup_fb(plane, old_fb, plane_state);
>  	}
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 5ba5792..813a066 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -437,7 +437,8 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>  
>  	if (plane_funcs->prepare_fb && plane_state->fb &&
>  	    plane_state->fb != old_fb) {
> -		ret = plane_funcs->prepare_fb(plane, plane_state->fb);
> +		ret = plane_funcs->prepare_fb(plane, plane_state->fb,
> +					      plane_state);
>  		if (ret)
>  			goto out;
>  	}
> @@ -487,7 +488,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>  	}
>  
>  	if (plane_funcs->cleanup_fb && old_fb)
> -		plane_funcs->cleanup_fb(plane, old_fb);
> +		plane_funcs->cleanup_fb(plane, old_fb, plane_state);
>  out:
>  	if (plane_state) {
>  		if (plane->funcs->atomic_destroy_state)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3156d77..c54a6e9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11891,7 +11891,8 @@ static void intel_shared_dpll_init(struct drm_device *dev)
>   */
>  int
>  intel_prepare_plane_fb(struct drm_plane *plane,
> -		       struct drm_framebuffer *fb)
> +		       struct drm_framebuffer *fb,
> +		       struct drm_plane_state *new_state)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -11945,7 +11946,8 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>   */
>  void
>  intel_cleanup_plane_fb(struct drm_plane *plane,
> -		       struct drm_framebuffer *fb)
> +		       struct drm_framebuffer *fb,
> +		       struct drm_plane_state *old_state)
>  {
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 632df1c..c466d43 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -965,9 +965,11 @@ void intel_finish_page_flip(struct drm_device *dev, int pipe);
>  void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
>  void intel_check_page_flip(struct drm_device *dev, int pipe);
>  int intel_prepare_plane_fb(struct drm_plane *plane,
> -			   struct drm_framebuffer *fb);
> +			   struct drm_framebuffer *fb,
> +			   struct drm_plane_state *new_state);
>  void intel_cleanup_plane_fb(struct drm_plane *plane,
> -			    struct drm_framebuffer *fb);
> +			    struct drm_framebuffer *fb,
> +			    struct drm_plane_state *old_state);
>  int intel_plane_atomic_get_property(struct drm_plane *plane,
>  				    const struct drm_plane_state *state,
>  				    struct drm_property *property,
> diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> index cde2500..8105c46 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c
> @@ -83,7 +83,8 @@ static const struct drm_plane_funcs mdp4_plane_funcs = {
>  };
>  
>  static int mdp4_plane_prepare_fb(struct drm_plane *plane,
> -		struct drm_framebuffer *fb)
> +		struct drm_framebuffer *fb,
> +		struct drm_plane_state *new_state)
>  {
>  	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
>  	struct mdp4_kms *mdp4_kms = get_kms(plane);
> @@ -93,7 +94,8 @@ static int mdp4_plane_prepare_fb(struct drm_plane *plane,
>  }
>  
>  static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
> -		struct drm_framebuffer *fb)
> +		struct drm_framebuffer *fb,
> +		struct drm_plane_state *old_state)
>  {
>  	struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
>  	struct mdp4_kms *mdp4_kms = get_kms(plane);
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 1a52522..9f97d2f 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -472,13 +472,15 @@ static const struct drm_plane_funcs tegra_primary_plane_funcs = {
>  };
>  
>  static int tegra_plane_prepare_fb(struct drm_plane *plane,
> -				  struct drm_framebuffer *fb)
> +				  struct drm_framebuffer *fb,
> +				  struct drm_plane_state *new_state)
>  {
>  	return 0;
>  }
>  
>  static void tegra_plane_cleanup_fb(struct drm_plane *plane,
> -				   struct drm_framebuffer *fb)
> +				   struct drm_framebuffer *fb,
> +				   struct drm_plane_state *old_fb)
>  {
>  }
>  
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 31c11d3..a0a72b2 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -59,9 +59,11 @@ extern int drm_crtc_init(struct drm_device *dev,
>   */
>  struct drm_plane_helper_funcs {
>  	int (*prepare_fb)(struct drm_plane *plane,
> -			  struct drm_framebuffer *fb);
> +			  struct drm_framebuffer *fb,
> +			  struct drm_plane_state *new_state);
>  	void (*cleanup_fb)(struct drm_plane *plane,
> -			   struct drm_framebuffer *fb);
> +			   struct drm_framebuffer *fb,
> +			   struct drm_plane_state *old_state);
>  
>  	int (*atomic_check)(struct drm_plane *plane,
>  			    struct drm_plane_state *state);
> -- 
> 2.3.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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-03-02 15:49 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-02 14:43 [PATCH 0/5] Skylake 90/270 display rotation Tvrtko Ursulin
2015-03-02 14:43 ` [PATCH 1/5] drm: Pass in new and old plane state to prepare_fb and cleanup_fb Tvrtko Ursulin
2015-03-02 15:51   ` Daniel Vetter [this message]
2015-03-02 14:43 ` [PATCH 2/5] drm/i915/skl: Extract tile height code into a helper function Tvrtko Ursulin
2015-03-02 14:43 ` [PATCH 3/5] drm/i915/skl: Support secondary (rotated) frame buffer mapping Tvrtko Ursulin
2015-03-02 18:21   ` Daniel Vetter
2015-03-03  9:59     ` Tvrtko Ursulin
2015-03-04 14:43       ` Daniel Vetter
2015-03-04 15:04         ` Tvrtko Ursulin
2015-03-02 14:43 ` [PATCH 4/5] drm/i915/skl: Query display address through a wrapper Tvrtko Ursulin
2015-03-02 14:43 ` [PATCH 5/5] drm/i915/skl: Take 90/270 rotation into account in watermark calculations Tvrtko Ursulin

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=20150302155104.GA24485@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=tvrtko.ursulin@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox