* Re: [PATCH 1/5] drm: Pass in new and old plane state to prepare_fb and cleanup_fb
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
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Vetter @ 2015-03-02 15:51 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Daniel Vetter, Intel-gfx, dri-devel
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
^ permalink raw reply [flat|nested] 2+ messages in thread