* [PATCH 0/7] i915 atomic plane helper conversion
@ 2014-11-13 18:43 Matt Roper
2014-11-13 18:43 ` [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper
` (7 more replies)
0 siblings, 8 replies; 29+ messages in thread
From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
This series takes another step along the path to atomic modeset / nuclear
pageflip by transitioning the i915 driver to the new atomic plane helpers
provided by Daniel Vetter in:
http://lists.freedesktop.org/archives/dri-devel/2014-November/071623.html
Previous work by Gustavo Padovan had already split most of the i915 plane
internals into separate prepare/check/commit steps, so a lot of the heavy
lifting was already done. This patch series sets up the hooks expected by the
atomic plane helpers, replaces the existing .update_plane() and
.disable_plane() entrypoints with the helpers, and does some general cleanup.
Patches #5 and #6 are the meat here; the other five patches are minor
refactoring and cleanup work.
I've tested this series on IVB with the plane-related i-g-t tests and some very
casual X usage.
Matt Roper (7):
drm/i915: Make intel_plane_state subclass drm_plane_state
drm/i915: Allow intel_plane_disable() to operate on all plane types
drm/i915: Clarify sprite plane function names
drm/i915: Make intel_crtc_has_pending_flip() non-static
drm/i915: Prepare for atomic plane helpers
drm/i915: Switch plane handling to atomic helpers
drm/i915: Drop unused position fields
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/intel_atomic.c | 217 ++++++++++++++++++++++
drivers/gpu/drm/i915/intel_display.c | 344 +++++++++--------------------------
drivers/gpu/drm/i915/intel_drv.h | 28 ++-
drivers/gpu/drm/i915/intel_sprite.c | 170 +++--------------
5 files changed, 351 insertions(+), 409 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_atomic.c
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 21:25 ` Bob Paauwe 2014-11-13 18:43 ` [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types Matt Roper ` (6 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++-------------- drivers/gpu/drm/i915/intel_drv.h | 3 +-- drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++-------- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 73e6656..a9f90b8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11553,8 +11553,8 @@ static int intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->crtc; - struct drm_framebuffer *fb = state->fb; + struct drm_crtc *crtc = state->base.crtc; + struct drm_framebuffer *fb = state->base.fb; struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; @@ -11570,8 +11570,8 @@ static int intel_prepare_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->crtc; - struct drm_framebuffer *fb = state->fb; + struct drm_crtc *crtc = state->base.crtc; + struct drm_framebuffer *fb = state->base.fb; struct drm_device *dev = crtc->dev; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; @@ -11606,8 +11606,8 @@ static void intel_commit_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->crtc; - struct drm_framebuffer *fb = state->fb; + struct drm_crtc *crtc = state->base.crtc; + struct drm_framebuffer *fb = state->base.fb; struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -11707,8 +11707,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int ret; - state.crtc = crtc; - state.fb = fb; + state.base.crtc = crtc; + state.base.fb = fb; /* sample coordinates in 16.16 fixed point */ state.src.x1 = src_x; @@ -11820,9 +11820,9 @@ static int intel_check_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->crtc; + struct drm_crtc *crtc = state->base.crtc; struct drm_device *dev = crtc->dev; - struct drm_framebuffer *fb = state->fb; + struct drm_framebuffer *fb = state->base.fb; struct drm_rect *dest = &state->dst; struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; @@ -11876,8 +11876,8 @@ static int intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct drm_crtc *crtc = state->crtc; - struct drm_framebuffer *fb = state->fb; + struct drm_crtc *crtc = state->base.crtc; + struct drm_framebuffer *fb = state->base.fb; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); @@ -11922,8 +11922,8 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, struct intel_plane_state state; int ret; - state.crtc = crtc; - state.fb = fb; + state.base.crtc = crtc; + state.base.fb = fb; /* sample coordinates in 16.16 fixed point */ state.src.x1 = src_x; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8432ae2..bd5ef4e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -244,8 +244,7 @@ typedef struct dpll { } intel_clock_t; struct intel_plane_state { - struct drm_crtc *crtc; - struct drm_framebuffer *fb; + struct drm_plane_state base; struct drm_rect src; struct drm_rect dst; struct drm_rect clip; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 7d9c340..fc96d13 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1096,9 +1096,9 @@ static int intel_check_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { - struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); + struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); struct intel_plane *intel_plane = to_intel_plane(plane); - struct drm_framebuffer *fb = state->fb; + struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; @@ -1262,11 +1262,11 @@ intel_prepare_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_device *dev = plane->dev; - struct drm_crtc *crtc = state->crtc; + struct drm_crtc *crtc = state->base.crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *fb = state->fb; + struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_plane->obj; int ret; @@ -1297,11 +1297,11 @@ intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) { struct drm_device *dev = plane->dev; - struct drm_crtc *crtc = state->crtc; + struct drm_crtc *crtc = state->base.crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_plane *intel_plane = to_intel_plane(plane); enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *fb = state->fb; + struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct drm_i915_gem_object *old_obj = intel_plane->obj; int crtc_x, crtc_y; @@ -1391,8 +1391,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int ret; - state.crtc = crtc; - state.fb = fb; + state.base.crtc = crtc; + state.base.fb = fb; /* sample coordinates in 16.16 fixed point */ state.src.x1 = src_x; -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state 2014-11-13 18:43 ` [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper @ 2014-11-13 21:25 ` Bob Paauwe 0 siblings, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 21:25 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:20 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 28 ++++++++++++++-------------- > drivers/gpu/drm/i915/intel_drv.h | 3 +-- > drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++-------- > 3 files changed, 23 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 73e6656..a9f90b8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11553,8 +11553,8 @@ static int > intel_check_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_crtc *crtc = state->crtc; > - struct drm_framebuffer *fb = state->fb; > + struct drm_crtc *crtc = state->base.crtc; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > struct drm_rect *src = &state->src; > const struct drm_rect *clip = &state->clip; > @@ -11570,8 +11570,8 @@ static int > intel_prepare_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_crtc *crtc = state->crtc; > - struct drm_framebuffer *fb = state->fb; > + struct drm_crtc *crtc = state->base.crtc; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_device *dev = crtc->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum pipe pipe = intel_crtc->pipe; > @@ -11606,8 +11606,8 @@ static void > intel_commit_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_crtc *crtc = state->crtc; > - struct drm_framebuffer *fb = state->fb; > + struct drm_crtc *crtc = state->base.crtc; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -11707,8 +11707,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int ret; > > - state.crtc = crtc; > - state.fb = fb; > + state.base.crtc = crtc; > + state.base.fb = fb; > > /* sample coordinates in 16.16 fixed point */ > state.src.x1 = src_x; > @@ -11820,9 +11820,9 @@ static int > intel_check_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_crtc *crtc = state->crtc; > + struct drm_crtc *crtc = state->base.crtc; > struct drm_device *dev = crtc->dev; > - struct drm_framebuffer *fb = state->fb; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_rect *dest = &state->dst; > struct drm_rect *src = &state->src; > const struct drm_rect *clip = &state->clip; > @@ -11876,8 +11876,8 @@ static int > intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct drm_crtc *crtc = state->crtc; > - struct drm_framebuffer *fb = state->fb; > + struct drm_crtc *crtc = state->base.crtc; > + struct drm_framebuffer *fb = state->base.fb; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > @@ -11922,8 +11922,8 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > struct intel_plane_state state; > int ret; > > - state.crtc = crtc; > - state.fb = fb; > + state.base.crtc = crtc; > + state.base.fb = fb; > > /* sample coordinates in 16.16 fixed point */ > state.src.x1 = src_x; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 8432ae2..bd5ef4e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -244,8 +244,7 @@ typedef struct dpll { > } intel_clock_t; > > struct intel_plane_state { > - struct drm_crtc *crtc; > - struct drm_framebuffer *fb; > + struct drm_plane_state base; > struct drm_rect src; > struct drm_rect dst; > struct drm_rect clip; > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 7d9c340..fc96d13 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1096,9 +1096,9 @@ static int > intel_check_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > - struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); > + struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > - struct drm_framebuffer *fb = state->fb; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > @@ -1262,11 +1262,11 @@ intel_prepare_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_device *dev = plane->dev; > - struct drm_crtc *crtc = state->crtc; > + struct drm_crtc *crtc = state->base.crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *fb = state->fb; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct drm_i915_gem_object *old_obj = intel_plane->obj; > int ret; > @@ -1297,11 +1297,11 @@ intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > struct drm_device *dev = plane->dev; > - struct drm_crtc *crtc = state->crtc; > + struct drm_crtc *crtc = state->base.crtc; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_plane *intel_plane = to_intel_plane(plane); > enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *fb = state->fb; > + struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > struct drm_i915_gem_object *old_obj = intel_plane->obj; > int crtc_x, crtc_y; > @@ -1391,8 +1391,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int ret; > > - state.crtc = crtc; > - state.fb = fb; > + state.base.crtc = crtc; > + state.base.fb = fb; > > /* sample coordinates in 16.16 fixed point */ > state.src.x1 = src_x; _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper 2014-11-13 18:43 ` [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 19:11 ` Bob Paauwe 2014-11-13 18:43 ` [PATCH 3/7] drm/i915: Clarify sprite plane function names Matt Roper ` (5 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel We'll want to call this from the type-agnostic atomic plane helper hooks. Since it's not sprite-specific anymore, more it to intel_display.c as well. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a9f90b8..c6598e9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) spin_unlock_irq(&dev->event_lock); } } + +void intel_plane_disable(struct drm_plane *plane) +{ + if (!plane->crtc || !plane->fb) + return; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + intel_primary_plane_disable(plane); + break; + case DRM_PLANE_TYPE_CURSOR: + intel_cursor_plane_disable(plane); + break; + case DRM_PLANE_TYPE_OVERLAY: + intel_disable_plane(plane); + break; + default: + WARN(1, "Unknown plane type"); + } +} + diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bd5ef4e..df1420b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); 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); +void intel_plane_disable(struct drm_plane *plane); /* shared dpll functions */ struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, struct drm_property *prop, uint64_t val); int intel_plane_restore(struct drm_plane *plane); -void intel_plane_disable(struct drm_plane *plane); int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv); int intel_sprite_get_colorkey(struct drm_device *dev, void *data, @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, bool intel_pipe_update_start(struct intel_crtc *crtc, uint32_t *start_vbl_count); void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); +int intel_disable_plane(struct drm_plane *plane); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index fc96d13..115acd3 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, return 0; } -static int +int intel_disable_plane(struct drm_plane *plane) { struct drm_device *dev = plane->dev; @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) intel_plane->src_w, intel_plane->src_h); } -void intel_plane_disable(struct drm_plane *plane) -{ - if (!plane->crtc || !plane->fb) - return; - - intel_disable_plane(plane); -} - static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, .disable_plane = intel_disable_plane, -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 18:43 ` [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types Matt Roper @ 2014-11-13 19:11 ` Bob Paauwe 2014-11-13 19:12 ` Matt Roper 2014-11-13 19:23 ` Ville Syrjälä 0 siblings, 2 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 19:11 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:21 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > We'll want to call this from the type-agnostic atomic plane helper > hooks. Since it's not sprite-specific anymore, more it to > intel_display.c as well. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > 3 files changed, 24 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a9f90b8..c6598e9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > spin_unlock_irq(&dev->event_lock); > } > } > + > +void intel_plane_disable(struct drm_plane *plane) > +{ > + if (!plane->crtc || !plane->fb) > + return; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + intel_primary_plane_disable(plane); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + intel_cursor_plane_disable(plane); > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + intel_disable_plane(plane); > + break; > + default: > + WARN(1, "Unknown plane type"); > + } > +} > + > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index bd5ef4e..df1420b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > 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); > +void intel_plane_disable(struct drm_plane *plane); > > /* shared dpll functions */ > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > struct drm_property *prop, > uint64_t val); > int intel_plane_restore(struct drm_plane *plane); > -void intel_plane_disable(struct drm_plane *plane); > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > struct drm_file *file_priv); > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > bool intel_pipe_update_start(struct intel_crtc *crtc, > uint32_t *start_vbl_count); > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > +int intel_disable_plane(struct drm_plane *plane); Would it make sense to rename this to intel_sprite_plane_disable() as part of this? It would be more consistent with the cursor and primary plane naming conventions and likely avoid some confusion with the intel_plane_disable() function. > > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index fc96d13..115acd3 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > return 0; > } > > -static int > +int > intel_disable_plane(struct drm_plane *plane) > { > struct drm_device *dev = plane->dev; > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > intel_plane->src_w, intel_plane->src_h); > } > > -void intel_plane_disable(struct drm_plane *plane) > -{ > - if (!plane->crtc || !plane->fb) > - return; > - > - intel_disable_plane(plane); > -} > - > static const struct drm_plane_funcs intel_plane_funcs = { > .update_plane = intel_update_plane, > .disable_plane = intel_disable_plane, _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 19:11 ` Bob Paauwe @ 2014-11-13 19:12 ` Matt Roper 2014-11-13 19:44 ` Bob Paauwe 2014-11-13 19:23 ` Ville Syrjälä 1 sibling, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 19:12 UTC (permalink / raw) To: Bob Paauwe; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > On Thu, 13 Nov 2014 10:43:21 -0800 > Matt Roper <matthew.d.roper@intel.com> wrote: > > > We'll want to call this from the type-agnostic atomic plane helper > > hooks. Since it's not sprite-specific anymore, more it to > > intel_display.c as well. > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a9f90b8..c6598e9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > spin_unlock_irq(&dev->event_lock); > > } > > } > > + > > +void intel_plane_disable(struct drm_plane *plane) > > +{ > > + if (!plane->crtc || !plane->fb) > > + return; > > + > > + switch (plane->type) { > > + case DRM_PLANE_TYPE_PRIMARY: > > + intel_primary_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_CURSOR: > > + intel_cursor_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_OVERLAY: > > + intel_disable_plane(plane); > > + break; > > + default: > > + WARN(1, "Unknown plane type"); > > + } > > +} > > + > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index bd5ef4e..df1420b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > 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); > > +void intel_plane_disable(struct drm_plane *plane); > > > > /* shared dpll functions */ > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > struct drm_property *prop, > > uint64_t val); > > int intel_plane_restore(struct drm_plane *plane); > > -void intel_plane_disable(struct drm_plane *plane); > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > uint32_t *start_vbl_count); > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > +int intel_disable_plane(struct drm_plane *plane); > > Would it make sense to rename this to intel_sprite_plane_disable() as > part of this? It would be more consistent with the cursor and primary > plane naming conventions and likely avoid some confusion with the > intel_plane_disable() function. > Yeah, that happens in the next patch of the series (along with some other naming cleanup). I tripped over the confusing names too many times while working on this... Matt > > > > /* intel_tv.c */ > > void intel_tv_init(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index fc96d13..115acd3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > return 0; > > } > > > > -static int > > +int > > intel_disable_plane(struct drm_plane *plane) > > { > > struct drm_device *dev = plane->dev; > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > intel_plane->src_w, intel_plane->src_h); > > } > > > > -void intel_plane_disable(struct drm_plane *plane) > > -{ > > - if (!plane->crtc || !plane->fb) > > - return; > > - > > - intel_disable_plane(plane); > > -} > > - > > static const struct drm_plane_funcs intel_plane_funcs = { > > .update_plane = intel_update_plane, > > .disable_plane = intel_disable_plane, > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 19:12 ` Matt Roper @ 2014-11-13 19:44 ` Bob Paauwe 0 siblings, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 19:44 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 11:12:01 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > > On Thu, 13 Nov 2014 10:43:21 -0800 > > Matt Roper <matthew.d.roper@intel.com> wrote: > > > > > We'll want to call this from the type-agnostic atomic plane helper > > > hooks. Since it's not sprite-specific anymore, more it to > > > intel_display.c as well. > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index a9f90b8..c6598e9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > > spin_unlock_irq(&dev->event_lock); > > > } > > > } > > > + > > > +void intel_plane_disable(struct drm_plane *plane) > > > +{ > > > + if (!plane->crtc || !plane->fb) > > > + return; > > > + > > > + switch (plane->type) { > > > + case DRM_PLANE_TYPE_PRIMARY: > > > + intel_primary_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_CURSOR: > > > + intel_cursor_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_OVERLAY: > > > + intel_disable_plane(plane); > > > + break; > > > + default: > > > + WARN(1, "Unknown plane type"); > > > + } > > > +} > > > + > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index bd5ef4e..df1420b 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > > 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); > > > +void intel_plane_disable(struct drm_plane *plane); > > > > > > /* shared dpll functions */ > > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > > struct drm_property *prop, > > > uint64_t val); > > > int intel_plane_restore(struct drm_plane *plane); > > > -void intel_plane_disable(struct drm_plane *plane); > > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > > struct drm_file *file_priv); > > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > > uint32_t *start_vbl_count); > > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > > +int intel_disable_plane(struct drm_plane *plane); > > > > Would it make sense to rename this to intel_sprite_plane_disable() as > > part of this? It would be more consistent with the cursor and primary > > plane naming conventions and likely avoid some confusion with the > > intel_plane_disable() function. > > > > Yeah, that happens in the next patch of the series (along with some > other naming cleanup). I tripped over the confusing names too many > times while working on this... > > > Matt I tripped over it on my first pass reading through this which is why I think it might be better as part of this patch. But being part of the next patch is fine too. Bob > > > > > > > > /* intel_tv.c */ > > > void intel_tv_init(struct drm_device *dev); > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index fc96d13..115acd3 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > > return 0; > > > } > > > > > > -static int > > > +int > > > intel_disable_plane(struct drm_plane *plane) > > > { > > > struct drm_device *dev = plane->dev; > > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > > intel_plane->src_w, intel_plane->src_h); > > > } > > > > > > -void intel_plane_disable(struct drm_plane *plane) > > > -{ > > > - if (!plane->crtc || !plane->fb) > > > - return; > > > - > > > - intel_disable_plane(plane); > > > -} > > > - > > > static const struct drm_plane_funcs intel_plane_funcs = { > > > .update_plane = intel_update_plane, > > > .disable_plane = intel_disable_plane, > > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 19:11 ` Bob Paauwe 2014-11-13 19:12 ` Matt Roper @ 2014-11-13 19:23 ` Ville Syrjälä 2014-11-13 22:15 ` Matt Roper 1 sibling, 1 reply; 29+ messages in thread From: Ville Syrjälä @ 2014-11-13 19:23 UTC (permalink / raw) To: Bob Paauwe; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > On Thu, 13 Nov 2014 10:43:21 -0800 > Matt Roper <matthew.d.roper@intel.com> wrote: > > > We'll want to call this from the type-agnostic atomic plane helper > > hooks. Since it's not sprite-specific anymore, more it to > > intel_display.c as well. > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a9f90b8..c6598e9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > spin_unlock_irq(&dev->event_lock); > > } > > } > > + > > +void intel_plane_disable(struct drm_plane *plane) > > +{ > > + if (!plane->crtc || !plane->fb) > > + return; > > + > > + switch (plane->type) { > > + case DRM_PLANE_TYPE_PRIMARY: > > + intel_primary_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_CURSOR: > > + intel_cursor_plane_disable(plane); > > + break; > > + case DRM_PLANE_TYPE_OVERLAY: > > + intel_disable_plane(plane); > > + break; > > + default: > > + WARN(1, "Unknown plane type"); > > + } > > +} > > + > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index bd5ef4e..df1420b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > 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); > > +void intel_plane_disable(struct drm_plane *plane); > > > > /* shared dpll functions */ > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > struct drm_property *prop, > > uint64_t val); > > int intel_plane_restore(struct drm_plane *plane); > > -void intel_plane_disable(struct drm_plane *plane); > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > struct drm_file *file_priv); > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > uint32_t *start_vbl_count); > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > +int intel_disable_plane(struct drm_plane *plane); > > Would it make sense to rename this to intel_sprite_plane_disable() as > part of this? It would be more consistent with the cursor and primary > plane naming conventions and likely avoid some confusion with the > intel_plane_disable() function. Hmm. Why do we even still have some kind of disable hook in the atomic age? I would expect to just have .commit() or somesuch thing. But is there's still some need for a disable hook, shouldn't we just call the .disable_plane() hook of drm_plane? I guess I should really go and read some this new helper stuff... > > > > /* intel_tv.c */ > > void intel_tv_init(struct drm_device *dev); > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index fc96d13..115acd3 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > return 0; > > } > > > > -static int > > +int > > intel_disable_plane(struct drm_plane *plane) > > { > > struct drm_device *dev = plane->dev; > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > intel_plane->src_w, intel_plane->src_h); > > } > > > > -void intel_plane_disable(struct drm_plane *plane) > > -{ > > - if (!plane->crtc || !plane->fb) > > - return; > > - > > - intel_disable_plane(plane); > > -} > > - > > static const struct drm_plane_funcs intel_plane_funcs = { > > .update_plane = intel_update_plane, > > .disable_plane = intel_disable_plane, > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types 2014-11-13 19:23 ` Ville Syrjälä @ 2014-11-13 22:15 ` Matt Roper 0 siblings, 0 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 22:15 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 09:23:02PM +0200, Ville Syrjälä wrote: > On Thu, Nov 13, 2014 at 11:11:38AM -0800, Bob Paauwe wrote: > > On Thu, 13 Nov 2014 10:43:21 -0800 > > Matt Roper <matthew.d.roper@intel.com> wrote: > > > > > We'll want to call this from the type-agnostic atomic plane helper > > > hooks. Since it's not sprite-specific anymore, more it to > > > intel_display.c as well. > > > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > > > drivers/gpu/drm/i915/intel_sprite.c | 10 +--------- > > > 3 files changed, 24 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index a9f90b8..c6598e9 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -13679,3 +13679,24 @@ void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file) > > > spin_unlock_irq(&dev->event_lock); > > > } > > > } > > > + > > > +void intel_plane_disable(struct drm_plane *plane) > > > +{ > > > + if (!plane->crtc || !plane->fb) > > > + return; > > > + > > > + switch (plane->type) { > > > + case DRM_PLANE_TYPE_PRIMARY: > > > + intel_primary_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_CURSOR: > > > + intel_cursor_plane_disable(plane); > > > + break; > > > + case DRM_PLANE_TYPE_OVERLAY: > > > + intel_disable_plane(plane); > > > + break; > > > + default: > > > + WARN(1, "Unknown plane type"); > > > + } > > > +} > > > + > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index bd5ef4e..df1420b 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -920,6 +920,7 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane); > > > 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); > > > +void intel_plane_disable(struct drm_plane *plane); > > > > > > /* shared dpll functions */ > > > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); > > > @@ -1180,7 +1181,6 @@ int intel_plane_set_property(struct drm_plane *plane, > > > struct drm_property *prop, > > > uint64_t val); > > > int intel_plane_restore(struct drm_plane *plane); > > > -void intel_plane_disable(struct drm_plane *plane); > > > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > > > struct drm_file *file_priv); > > > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > @@ -1188,6 +1188,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > > > bool intel_pipe_update_start(struct intel_crtc *crtc, > > > uint32_t *start_vbl_count); > > > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > > > +int intel_disable_plane(struct drm_plane *plane); > > > > Would it make sense to rename this to intel_sprite_plane_disable() as > > part of this? It would be more consistent with the cursor and primary > > plane naming conventions and likely avoid some confusion with the > > intel_plane_disable() function. > > Hmm. Why do we even still have some kind of disable hook in the atomic > age? I would expect to just have .commit() or somesuch thing. > > But is there's still some need for a disable hook, shouldn't we just call > the .disable_plane() hook of drm_plane? I guess I should really go and > read some this new helper stuff... You're right that it does eventually come down to drm_plane_helper_commit() --> intel_plane_atomic_update() which should be able to program the plane to either a new state or off. To reuse as much of our existing code as possible and keep my changes minimal, I'm just implementing intel_plane_atomic_update() as: if (!plane->state->fb) intel_plane_disable(plane); else intel_plane->commit_plane(plane, intel_state); since we already had both of those functions implemented and working. intel_plane->commit_plane() doesn't expect to be called in the disabled case today (assumes crtc and/or fb are non-NULL), but it definitely seems reasonable/easy to tweak that and roll the disable logic directly into the lowest-level commit_plane in the future. I just wanted to keep things as simple as possible with minimal code changes for the initial patchset. Matt > > > > > > > /* intel_tv.c */ > > > void intel_tv_init(struct drm_device *dev); > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > > index fc96d13..115acd3 100644 > > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > > @@ -1425,7 +1425,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > > return 0; > > > } > > > > > > -static int > > > +int > > > intel_disable_plane(struct drm_plane *plane) > > > { > > > struct drm_device *dev = plane->dev; > > > @@ -1576,14 +1576,6 @@ int intel_plane_restore(struct drm_plane *plane) > > > intel_plane->src_w, intel_plane->src_h); > > > } > > > > > > -void intel_plane_disable(struct drm_plane *plane) > > > -{ > > > - if (!plane->crtc || !plane->fb) > > > - return; > > > - > > > - intel_disable_plane(plane); > > > -} > > > - > > > static const struct drm_plane_funcs intel_plane_funcs = { > > > .update_plane = intel_update_plane, > > > .disable_plane = intel_disable_plane, > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel OTC -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/7] drm/i915: Clarify sprite plane function names 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper 2014-11-13 18:43 ` [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper 2014-11-13 18:43 ` [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 21:26 ` [Intel-gfx] " Bob Paauwe 2014-11-13 18:43 ` [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static Matt Roper ` (4 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel A few of the sprite-related function names in i915 are very similar (intel_plane_disable() vs intel_disable_plane() or intel_enable_planes() vs intel_crtc_enable_planes()) and don't make it clear whether they only operate on sprite planes, or whether they also apply to all universal plane types. Rename a few functions to be more consistent with our function naming for primary/cursor planes or to clarify that they apply specifically to sprite planes: - s/intel_disable_plane/intel_sprite_plane_disable/ - s/intel_disable_planes/intel_disable_sprite_planes/ - s/intel_enable_planes/intel_enable_sprite_planes/ Also, drop the sprite-specific intel_destroy_plane() and just use the type-agnostic intel_plane_destroy() function. The extra 'disable' call that intel_destroy_plane() did is unnecessary since the plane will already be disabled due to framebuffer destruction by the point it gets called. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 12 ++++++------ drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_sprite.c | 14 +++----------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c6598e9..52ca8c4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4024,7 +4024,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc) } } -static void intel_enable_planes(struct drm_crtc *crtc) +static void intel_enable_sprite_planes(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; enum pipe pipe = to_intel_crtc(crtc)->pipe; @@ -4038,7 +4038,7 @@ static void intel_enable_planes(struct drm_crtc *crtc) } } -static void intel_disable_planes(struct drm_crtc *crtc) +static void intel_disable_sprite_planes(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; enum pipe pipe = to_intel_crtc(crtc)->pipe; @@ -4182,7 +4182,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; intel_enable_primary_hw_plane(crtc->primary, crtc); - intel_enable_planes(crtc); + intel_enable_sprite_planes(crtc); intel_crtc_update_cursor(crtc, true); intel_crtc_dpms_overlay(intel_crtc, true); @@ -4217,7 +4217,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc) intel_crtc_dpms_overlay(intel_crtc, false); intel_crtc_update_cursor(crtc, false); - intel_disable_planes(crtc); + intel_disable_sprite_planes(crtc); intel_disable_primary_hw_plane(crtc->primary, crtc); /* @@ -11744,7 +11744,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, } /* Common destruction function for both primary and cursor planes */ -static void intel_plane_destroy(struct drm_plane *plane) +void intel_plane_destroy(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); drm_plane_cleanup(plane); @@ -13693,7 +13693,7 @@ void intel_plane_disable(struct drm_plane *plane) intel_cursor_plane_disable(plane); break; case DRM_PLANE_TYPE_OVERLAY: - intel_disable_plane(plane); + intel_sprite_plane_disable(plane); break; default: WARN(1, "Unknown plane type"); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index df1420b..4913b3f 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1181,6 +1181,7 @@ int intel_plane_set_property(struct drm_plane *plane, struct drm_property *prop, uint64_t val); int intel_plane_restore(struct drm_plane *plane); +void intel_plane_destroy(struct drm_plane *plane); int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv); int intel_sprite_get_colorkey(struct drm_device *dev, void *data, @@ -1188,7 +1189,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, bool intel_pipe_update_start(struct intel_crtc *crtc, uint32_t *start_vbl_count); void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); -int intel_disable_plane(struct drm_plane *plane); +int intel_sprite_plane_disable(struct drm_plane *plane); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 115acd3..9e6a72a 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1426,7 +1426,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, } int -intel_disable_plane(struct drm_plane *plane) +intel_sprite_plane_disable(struct drm_plane *plane) { struct drm_device *dev = plane->dev; struct intel_plane *intel_plane = to_intel_plane(plane); @@ -1469,14 +1469,6 @@ intel_disable_plane(struct drm_plane *plane) return 0; } -static void intel_destroy_plane(struct drm_plane *plane) -{ - struct intel_plane *intel_plane = to_intel_plane(plane); - intel_disable_plane(plane); - drm_plane_cleanup(plane); - kfree(intel_plane); -} - int intel_sprite_set_colorkey(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1578,8 +1570,8 @@ int intel_plane_restore(struct drm_plane *plane) static const struct drm_plane_funcs intel_plane_funcs = { .update_plane = intel_update_plane, - .disable_plane = intel_disable_plane, - .destroy = intel_destroy_plane, + .disable_plane = intel_sprite_plane_disable, + .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, }; -- 1.8.5.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 3/7] drm/i915: Clarify sprite plane function names 2014-11-13 18:43 ` [PATCH 3/7] drm/i915: Clarify sprite plane function names Matt Roper @ 2014-11-13 21:26 ` Bob Paauwe 0 siblings, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 21:26 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:22 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > A few of the sprite-related function names in i915 are very similar > (intel_plane_disable() vs intel_disable_plane() or > intel_enable_planes() vs intel_crtc_enable_planes()) and don't make it > clear whether they only operate on sprite planes, or whether they also > apply to all universal plane types. Rename a few functions to be more > consistent with our function naming for primary/cursor planes or to > clarify that they apply specifically to sprite planes: > > - s/intel_disable_plane/intel_sprite_plane_disable/ > - s/intel_disable_planes/intel_disable_sprite_planes/ > - s/intel_enable_planes/intel_enable_sprite_planes/ > > Also, drop the sprite-specific intel_destroy_plane() and just use > the type-agnostic intel_plane_destroy() function. The extra 'disable' > call that intel_destroy_plane() did is unnecessary since the plane will > already be disabled due to framebuffer destruction by the point it gets > called. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 12 ++++++------ > drivers/gpu/drm/i915/intel_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_sprite.c | 14 +++----------- > 3 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index c6598e9..52ca8c4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4024,7 +4024,7 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc) > } > } > > -static void intel_enable_planes(struct drm_crtc *crtc) > +static void intel_enable_sprite_planes(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > enum pipe pipe = to_intel_crtc(crtc)->pipe; > @@ -4038,7 +4038,7 @@ static void intel_enable_planes(struct drm_crtc *crtc) > } > } > > -static void intel_disable_planes(struct drm_crtc *crtc) > +static void intel_disable_sprite_planes(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > enum pipe pipe = to_intel_crtc(crtc)->pipe; > @@ -4182,7 +4182,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) > int pipe = intel_crtc->pipe; > > intel_enable_primary_hw_plane(crtc->primary, crtc); > - intel_enable_planes(crtc); > + intel_enable_sprite_planes(crtc); > intel_crtc_update_cursor(crtc, true); > intel_crtc_dpms_overlay(intel_crtc, true); > > @@ -4217,7 +4217,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc) > > intel_crtc_dpms_overlay(intel_crtc, false); > intel_crtc_update_cursor(crtc, false); > - intel_disable_planes(crtc); > + intel_disable_sprite_planes(crtc); > intel_disable_primary_hw_plane(crtc->primary, crtc); > > /* > @@ -11744,7 +11744,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > } > > /* Common destruction function for both primary and cursor planes */ > -static void intel_plane_destroy(struct drm_plane *plane) > +void intel_plane_destroy(struct drm_plane *plane) > { > struct intel_plane *intel_plane = to_intel_plane(plane); > drm_plane_cleanup(plane); > @@ -13693,7 +13693,7 @@ void intel_plane_disable(struct drm_plane *plane) > intel_cursor_plane_disable(plane); > break; > case DRM_PLANE_TYPE_OVERLAY: > - intel_disable_plane(plane); > + intel_sprite_plane_disable(plane); > break; > default: > WARN(1, "Unknown plane type"); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index df1420b..4913b3f 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1181,6 +1181,7 @@ int intel_plane_set_property(struct drm_plane *plane, > struct drm_property *prop, > uint64_t val); > int intel_plane_restore(struct drm_plane *plane); > +void intel_plane_destroy(struct drm_plane *plane); > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > struct drm_file *file_priv); > int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > @@ -1188,7 +1189,7 @@ int intel_sprite_get_colorkey(struct drm_device *dev, void *data, > bool intel_pipe_update_start(struct intel_crtc *crtc, > uint32_t *start_vbl_count); > void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count); > -int intel_disable_plane(struct drm_plane *plane); > +int intel_sprite_plane_disable(struct drm_plane *plane); > > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 115acd3..9e6a72a 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1426,7 +1426,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > } > > int > -intel_disable_plane(struct drm_plane *plane) > +intel_sprite_plane_disable(struct drm_plane *plane) > { > struct drm_device *dev = plane->dev; > struct intel_plane *intel_plane = to_intel_plane(plane); > @@ -1469,14 +1469,6 @@ intel_disable_plane(struct drm_plane *plane) > return 0; > } > > -static void intel_destroy_plane(struct drm_plane *plane) > -{ > - struct intel_plane *intel_plane = to_intel_plane(plane); > - intel_disable_plane(plane); > - drm_plane_cleanup(plane); > - kfree(intel_plane); > -} > - > int intel_sprite_set_colorkey(struct drm_device *dev, void *data, > struct drm_file *file_priv) > { > @@ -1578,8 +1570,8 @@ int intel_plane_restore(struct drm_plane *plane) > > static const struct drm_plane_funcs intel_plane_funcs = { > .update_plane = intel_update_plane, > - .disable_plane = intel_disable_plane, > - .destroy = intel_destroy_plane, > + .disable_plane = intel_sprite_plane_disable, > + .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > }; > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper ` (2 preceding siblings ...) 2014-11-13 18:43 ` [PATCH 3/7] drm/i915: Clarify sprite plane function names Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 21:27 ` Bob Paauwe 2014-11-13 18:43 ` [PATCH 5/7] drm/i915: Prepare for atomic plane helpers Matt Roper ` (3 subsequent siblings) 7 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel We'll want to use this from the atomic plane helpers, so ensure it can be called outside intel_display.c. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 52ca8c4..2d4ef04 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2833,7 +2833,7 @@ intel_finish_fb(struct drm_framebuffer *old_fb) return ret; } -static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) +bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4913b3f..49358c8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -921,6 +921,7 @@ 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); void intel_plane_disable(struct drm_plane *plane); +bool intel_crtc_has_pending_flip(struct drm_crtc *crtc); /* shared dpll functions */ struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); -- 1.8.5.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static 2014-11-13 18:43 ` [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static Matt Roper @ 2014-11-13 21:27 ` Bob Paauwe 0 siblings, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 21:27 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:23 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > We'll want to use this from the atomic plane helpers, so ensure it can > be called outside intel_display.c. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 52ca8c4..2d4ef04 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2833,7 +2833,7 @@ intel_finish_fb(struct drm_framebuffer *old_fb) > return ret; > } > > -static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) > +bool intel_crtc_has_pending_flip(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 4913b3f..49358c8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -921,6 +921,7 @@ 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); > void intel_plane_disable(struct drm_plane *plane); > +bool intel_crtc_has_pending_flip(struct drm_crtc *crtc); > > /* shared dpll functions */ > struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/7] drm/i915: Prepare for atomic plane helpers 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper ` (3 preceding siblings ...) 2014-11-13 18:43 ` [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 19:46 ` Bob Paauwe 2014-11-13 22:51 ` [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) Matt Roper 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper ` (2 subsequent siblings) 7 siblings, 2 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Add the new driver entrypoints that will be called by the atomic plane helpers. This patch does not actually switch over to the new plane helpers yet, so there should be no functional change here. Also note that although plane programming was already split into check/prepare/commit steps, some of the semantics of those individual functions will need to change slightly when we do make the jump so that they match the behavior the plane helpers expect. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic.c | 220 +++++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 20 +++- drivers/gpu/drm/i915/intel_drv.h | 15 +++ drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 5 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 891e584..8b67617 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -63,6 +63,7 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ + intel_atomic.o \ intel_crt.o \ intel_ddi.o \ intel_dp.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c new file mode 100644 index 0000000..2e9b56e --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -0,0 +1,220 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_plane_helper.h> +#include "intel_drv.h" + +/** + * intel_plane_duplicate_state - duplicate plane state + * @plane: drm plane + * + * Allocates and returns a copy of the plane state (both common and + * Intel-specific) for the specified plane. + */ +struct drm_plane_state * +intel_plane_duplicate_state(struct drm_plane *plane) +{ + struct intel_plane_state *state; + + if (plane->state) + state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL); + else + state = kzalloc(sizeof(*state), GFP_KERNEL); + + if (state && state->base.fb) + drm_framebuffer_reference(state->base.fb); + + return &state->base; +} + +/** + * intel_plane_destroy_state - destroy plane state + * @plane: drm plane + * + * Allocates and returns a copy of the plane state (both common and + * Intel-specific) for the specified plane. + */ +void +intel_plane_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + drm_atomic_helper_plane_destroy_state(plane, state); +} + + +/** + * intel_crtc_atomic_begin - Begins an atomic commit on a CRTC + * @crtc: drm crtc + * + * Prepares to write registers associated with the atomic commit of a CRTC + * by using vblank evasion to ensure that all register writes happen within + * the same vblank period. + */ +void intel_crtc_atomic_begin(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + intel_pipe_update_start(intel_crtc, &intel_crtc->atomic_vbl_count); +} + +/** + * intel_crtc_atomic_flush - Finishes an atomic commit on a CRTC + * @crtc: drm crtc + * + * Concludes the writing of registers for an atomic commit of a CRTC. + */ +void intel_crtc_atomic_flush(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + intel_pipe_update_end(intel_crtc, intel_crtc->atomic_vbl_count); +} + +static int intel_prepare_fb(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = plane->dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct drm_i915_gem_object *old_obj = intel_plane->obj; + enum pipe pipe = intel_plane->pipe; + unsigned front_bits = 0; + int ret = 0; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + front_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); + + if (plane->crtc) { + intel_crtc_wait_for_pending_flips(plane->crtc); + if (intel_crtc_has_pending_flip(plane->crtc)) { + DRM_ERROR("pipe is still busy with an old pageflip\n"); + return -EBUSY; + } + } + + break; + case DRM_PLANE_TYPE_OVERLAY: + front_bits = INTEL_FRONTBUFFER_SPRITE(pipe); + break; + case DRM_PLANE_TYPE_CURSOR: + front_bits = INTEL_FRONTBUFFER_CURSOR(pipe); + break; + } + + mutex_lock(&dev->struct_mutex); + + /* Note that this will apply the VT-d workaround for scanouts, + * which is more restrictive than required for sprites. (The + * primary plane requires 256KiB alignment with 64 PTE padding, + * the sprite planes only require 128KiB alignment and 32 PTE + * padding. + */ + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); + if (ret == 0) + i915_gem_track_fb(old_obj, obj, front_bits); + + if (plane->type == DRM_PLANE_TYPE_CURSOR && + INTEL_INFO(dev)->cursor_needs_physical) { + int align = IS_I830(dev) ? 16 * 1024 : 256; + + ret = i915_gem_object_attach_phys(obj, align); + if (ret) + DRM_DEBUG_KMS("failed to attach phys object\n"); + } + + mutex_unlock(&dev->struct_mutex); + + return ret; +} + +static void intel_cleanup_fb(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = plane->dev; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + + mutex_lock(&dev->struct_mutex); + intel_unpin_fb_obj(obj); + mutex_unlock(&dev->struct_mutex); +} + +static int intel_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); + struct intel_plane_state *intel_state = to_intel_plane_state(state); + + /* Disabling a plane is always okay */ + if (state->fb == NULL) + return 0; + + /* + * The original src/dest coordinates are stored in state->base, but + * we want to keep another copy internal to our driver that we can + * clip/modify ourselves. + */ + intel_state->src.x1 = state->src_x; + intel_state->src.y1 = state->src_y; + intel_state->src.x2 = state->src_x + state->src_w; + intel_state->src.y2 = state->src_y + state->src_h; + intel_state->dst.x1 = state->crtc_x; + intel_state->dst.y1 = state->crtc_y; + intel_state->dst.x2 = state->crtc_x + state->crtc_w; + intel_state->dst.y2 = state->crtc_y + state->crtc_h; + + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ + if (intel_crtc) { + intel_state->clip.x1 = 0; + intel_state->clip.y1 = 0; + intel_state->clip.x2 = + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; + intel_state->clip.y2 = + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; + } + + return intel_plane->check_plane(plane, intel_state); +} + +static void intel_plane_atomic_update(struct drm_plane *plane) +{ + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_plane_state *intel_state = + to_intel_plane_state(plane->state); + + if (!plane->state->fb) + intel_plane_disable(plane); + else + intel_plane->commit_plane(plane, intel_state); +} + +const struct drm_plane_helper_funcs intel_plane_helper_funcs = { + .prepare_fb = intel_prepare_fb, + .cleanup_fb = intel_cleanup_fb, + .atomic_check = intel_plane_atomic_check, + .atomic_update = intel_plane_atomic_update, +}; + diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2d4ef04..32a2b12 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9705,6 +9705,8 @@ out_hang: static struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, .load_lut = intel_crtc_load_lut, + .atomic_begin = intel_crtc_atomic_begin, + .atomic_flush = intel_crtc_atomic_flush, }; /** @@ -11747,6 +11749,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, void intel_plane_destroy(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); + intel_plane_destroy_state(plane, plane->state); drm_plane_cleanup(plane); kfree(intel_plane); } @@ -11755,7 +11758,10 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = { .update_plane = intel_primary_plane_setplane, .disable_plane = intel_primary_plane_disable, .destroy = intel_plane_destroy, - .set_property = intel_plane_set_property + .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, + }; static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, @@ -11769,11 +11775,14 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, if (primary == NULL) return NULL; + primary->base.state = intel_plane_duplicate_state(&primary->base); primary->can_scale = false; primary->max_downscale = 1; primary->pipe = pipe; primary->plane = pipe; primary->rotation = BIT(DRM_ROTATE_0); + primary->check_plane = intel_check_primary_plane; + primary->commit_plane = intel_commit_primary_plane; if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) primary->plane = !pipe; @@ -11802,6 +11811,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, primary->rotation); } + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs); + return &primary->base; } @@ -11957,6 +11968,8 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { .disable_plane = intel_cursor_plane_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, }; static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, @@ -11968,11 +11981,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, if (cursor == NULL) return NULL; + cursor->base.state = intel_plane_duplicate_state(&cursor->base); cursor->can_scale = false; cursor->max_downscale = 1; cursor->pipe = pipe; cursor->plane = pipe; cursor->rotation = BIT(DRM_ROTATE_0); + cursor->check_plane = intel_check_cursor_plane; + cursor->commit_plane = intel_commit_cursor_plane; drm_universal_plane_init(dev, &cursor->base, 0, &intel_cursor_plane_funcs, @@ -11992,6 +12008,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->rotation); } + drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs); + return &cursor->base; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 49358c8..1882de1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -466,6 +466,8 @@ struct intel_crtc { int scanline_offset; struct intel_mmio_flip mmio_flip; + + uint32_t atomic_vbl_count; }; struct intel_plane_wm_parameters { @@ -506,6 +508,10 @@ struct intel_plane { uint32_t src_w, uint32_t src_h); void (*disable_plane)(struct drm_plane *plane, struct drm_crtc *crtc); + int (*check_plane)(struct drm_plane *plane, + struct intel_plane_state *state); + void (*commit_plane)(struct drm_plane *plane, + struct intel_plane_state *state); int (*update_colorkey)(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key); void (*get_colorkey)(struct drm_plane *plane, @@ -536,6 +542,7 @@ struct cxsr_latency { #define to_intel_encoder(x) container_of(x, struct intel_encoder, base) #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base) #define to_intel_plane(x) container_of(x, struct intel_plane, base) +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base) #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL) struct intel_hdmi { @@ -1195,4 +1202,12 @@ int intel_sprite_plane_disable(struct drm_plane *plane); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); +/* intel_atomic.c */ +struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); +void intel_plane_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); +void intel_crtc_atomic_begin(struct drm_crtc *crtc); +void intel_crtc_atomic_flush(struct drm_crtc *crtc); +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; + #endif /* __INTEL_DRV_H__ */ diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9e6a72a..d3677c3 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -33,6 +33,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> #include <drm/drm_rect.h> +#include <drm/drm_plane_helper.h> #include "intel_drv.h" #include <drm/i915_drm.h> #include "i915_drv.h" @@ -1573,6 +1574,8 @@ static const struct drm_plane_funcs intel_plane_funcs = { .disable_plane = intel_sprite_plane_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, }; static uint32_t ilk_plane_formats[] = { @@ -1634,6 +1637,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) if (!intel_plane) return -ENOMEM; + intel_plane->base.state = + intel_plane_duplicate_state(&intel_plane->base); + switch (INTEL_INFO(dev)->gen) { case 5: case 6: @@ -1704,6 +1710,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane->pipe = pipe; intel_plane->plane = plane; intel_plane->rotation = BIT(DRM_ROTATE_0); + intel_plane->check_plane = intel_check_sprite_plane; + intel_plane->commit_plane = intel_commit_sprite_plane; possible_crtcs = (1 << pipe); ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, &intel_plane_funcs, @@ -1725,6 +1733,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) dev->mode_config.rotation_property, intel_plane->rotation); + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs); + out: return ret; } -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] drm/i915: Prepare for atomic plane helpers 2014-11-13 18:43 ` [PATCH 5/7] drm/i915: Prepare for atomic plane helpers Matt Roper @ 2014-11-13 19:46 ` Bob Paauwe 2014-11-13 21:31 ` Matt Roper 2014-11-13 22:51 ` [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) Matt Roper 1 sibling, 1 reply; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 19:46 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:24 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > Add the new driver entrypoints that will be called by the atomic plane > helpers. > > This patch does not actually switch over to the new plane helpers yet, > so there should be no functional change here. Also note that although > plane programming was already split into check/prepare/commit steps, > some of the semantics of those individual functions will need to change > slightly when we do make the jump so that they match the behavior the > plane helpers expect. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/intel_atomic.c | 220 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 20 +++- > drivers/gpu/drm/i915/intel_drv.h | 15 +++ > drivers/gpu/drm/i915/intel_sprite.c | 10 ++ > 5 files changed, 265 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/i915/intel_atomic.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 891e584..8b67617 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -63,6 +63,7 @@ i915-y += dvo_ch7017.o \ > dvo_ns2501.o \ > dvo_sil164.o \ > dvo_tfp410.o \ > + intel_atomic.o \ > intel_crt.o \ > intel_ddi.o \ > intel_dp.o \ > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > new file mode 100644 > index 0000000..2e9b56e > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -0,0 +1,220 @@ > +/* > + * Copyright © 2014 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_plane_helper.h> > +#include "intel_drv.h" > + > +/** > + * intel_plane_duplicate_state - duplicate plane state > + * @plane: drm plane > + * > + * Allocates and returns a copy of the plane state (both common and > + * Intel-specific) for the specified plane. > + */ > +struct drm_plane_state * > +intel_plane_duplicate_state(struct drm_plane *plane) > +{ > + struct intel_plane_state *state; > + > + if (plane->state) > + state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL); > + else > + state = kzalloc(sizeof(*state), GFP_KERNEL); > + > + if (state && state->base.fb) > + drm_framebuffer_reference(state->base.fb); > + > + return &state->base; > +} > + > +/** > + * intel_plane_destroy_state - destroy plane state > + * @plane: drm plane > + * > + * Allocates and returns a copy of the plane state (both common and > + * Intel-specific) for the specified plane. Comment should be updated for destroy. > + */ > +void > +intel_plane_destroy_state(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + drm_atomic_helper_plane_destroy_state(plane, state); > +} > + > + > +/** > + * intel_crtc_atomic_begin - Begins an atomic commit on a CRTC > + * @crtc: drm crtc > + * > + * Prepares to write registers associated with the atomic commit of a CRTC > + * by using vblank evasion to ensure that all register writes happen within > + * the same vblank period. > + */ > +void intel_crtc_atomic_begin(struct drm_crtc *crtc) > +{ > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + > + intel_pipe_update_start(intel_crtc, &intel_crtc->atomic_vbl_count); > +} > + > +/** > + * intel_crtc_atomic_flush - Finishes an atomic commit on a CRTC > + * @crtc: drm crtc > + * > + * Concludes the writing of registers for an atomic commit of a CRTC. > + */ > +void intel_crtc_atomic_flush(struct drm_crtc *crtc) > +{ > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + > + intel_pipe_update_end(intel_crtc, intel_crtc->atomic_vbl_count); > +} > + > +static int intel_prepare_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj = intel_plane->obj; > + enum pipe pipe = intel_plane->pipe; > + unsigned front_bits = 0; > + int ret = 0; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + front_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); > + > + if (plane->crtc) { > + intel_crtc_wait_for_pending_flips(plane->crtc); > + if (intel_crtc_has_pending_flip(plane->crtc)) { > + DRM_ERROR("pipe is still busy with an old pageflip\n"); > + return -EBUSY; > + } > + } > + > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + front_bits = INTEL_FRONTBUFFER_SPRITE(pipe); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + front_bits = INTEL_FRONTBUFFER_CURSOR(pipe); > + break; > + } > + > + mutex_lock(&dev->struct_mutex); > + > + /* Note that this will apply the VT-d workaround for scanouts, > + * which is more restrictive than required for sprites. (The > + * primary plane requires 256KiB alignment with 64 PTE padding, > + * the sprite planes only require 128KiB alignment and 32 PTE > + * padding. > + */ > + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > + if (ret == 0) > + i915_gem_track_fb(old_obj, obj, front_bits); > + > + if (plane->type == DRM_PLANE_TYPE_CURSOR && > + INTEL_INFO(dev)->cursor_needs_physical) { > + int align = IS_I830(dev) ? 16 * 1024 : 256; > + > + ret = i915_gem_object_attach_phys(obj, align); > + if (ret) > + DRM_DEBUG_KMS("failed to attach phys object\n"); > + } > + > + mutex_unlock(&dev->struct_mutex); > + > + return ret; > +} > + > +static void intel_cleanup_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + > + mutex_lock(&dev->struct_mutex); > + intel_unpin_fb_obj(obj); > + mutex_unlock(&dev->struct_mutex); > +} > + > +static int intel_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); > + struct intel_plane_state *intel_state = to_intel_plane_state(state); > + > + /* Disabling a plane is always okay */ > + if (state->fb == NULL) > + return 0; > + > + /* > + * The original src/dest coordinates are stored in state->base, but > + * we want to keep another copy internal to our driver that we can > + * clip/modify ourselves. > + */ > + intel_state->src.x1 = state->src_x; > + intel_state->src.y1 = state->src_y; > + intel_state->src.x2 = state->src_x + state->src_w; > + intel_state->src.y2 = state->src_y + state->src_h; > + intel_state->dst.x1 = state->crtc_x; > + intel_state->dst.y1 = state->crtc_y; > + intel_state->dst.x2 = state->crtc_x + state->crtc_w; > + intel_state->dst.y2 = state->crtc_y + state->crtc_h; > + > + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ > + if (intel_crtc) { > + intel_state->clip.x1 = 0; > + intel_state->clip.y1 = 0; > + intel_state->clip.x2 = > + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > + intel_state->clip.y2 = > + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; Should this be using intel_crtc->new_config here? I'm not entierly clear on how the crtc config/new_config would tie into the atomic state. > + } > + > + return intel_plane->check_plane(plane, intel_state); > +} > + > +static void intel_plane_atomic_update(struct drm_plane *plane) > +{ > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct intel_plane_state *intel_state = > + to_intel_plane_state(plane->state); > + > + if (!plane->state->fb) > + intel_plane_disable(plane); > + else > + intel_plane->commit_plane(plane, intel_state); > +} > + > +const struct drm_plane_helper_funcs intel_plane_helper_funcs = { > + .prepare_fb = intel_prepare_fb, > + .cleanup_fb = intel_cleanup_fb, > + .atomic_check = intel_plane_atomic_check, > + .atomic_update = intel_plane_atomic_update, > +}; > + > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2d4ef04..32a2b12 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9705,6 +9705,8 @@ out_hang: > static struct drm_crtc_helper_funcs intel_helper_funcs = { > .mode_set_base_atomic = intel_pipe_set_base_atomic, > .load_lut = intel_crtc_load_lut, > + .atomic_begin = intel_crtc_atomic_begin, > + .atomic_flush = intel_crtc_atomic_flush, > }; > > /** > @@ -11747,6 +11749,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > void intel_plane_destroy(struct drm_plane *plane) > { > struct intel_plane *intel_plane = to_intel_plane(plane); > + intel_plane_destroy_state(plane, plane->state); > drm_plane_cleanup(plane); > kfree(intel_plane); > } > @@ -11755,7 +11758,10 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = { > .update_plane = intel_primary_plane_setplane, > .disable_plane = intel_primary_plane_disable, > .destroy = intel_plane_destroy, > - .set_property = intel_plane_set_property > + .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > + > }; > > static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > @@ -11769,11 +11775,14 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > if (primary == NULL) > return NULL; > > + primary->base.state = intel_plane_duplicate_state(&primary->base); > primary->can_scale = false; > primary->max_downscale = 1; > primary->pipe = pipe; > primary->plane = pipe; > primary->rotation = BIT(DRM_ROTATE_0); > + primary->check_plane = intel_check_primary_plane; > + primary->commit_plane = intel_commit_primary_plane; > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > primary->plane = !pipe; > > @@ -11802,6 +11811,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > primary->rotation); > } > > + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs); > + > return &primary->base; > } > > @@ -11957,6 +11968,8 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { > .disable_plane = intel_cursor_plane_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > }; > > static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > @@ -11968,11 +11981,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > if (cursor == NULL) > return NULL; > > + cursor->base.state = intel_plane_duplicate_state(&cursor->base); > cursor->can_scale = false; > cursor->max_downscale = 1; > cursor->pipe = pipe; > cursor->plane = pipe; > cursor->rotation = BIT(DRM_ROTATE_0); > + cursor->check_plane = intel_check_cursor_plane; > + cursor->commit_plane = intel_commit_cursor_plane; > > drm_universal_plane_init(dev, &cursor->base, 0, > &intel_cursor_plane_funcs, > @@ -11992,6 +12008,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > cursor->rotation); > } > > + drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs); > + > return &cursor->base; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 49358c8..1882de1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -466,6 +466,8 @@ struct intel_crtc { > > int scanline_offset; > struct intel_mmio_flip mmio_flip; > + > + uint32_t atomic_vbl_count; > }; > > struct intel_plane_wm_parameters { > @@ -506,6 +508,10 @@ struct intel_plane { > uint32_t src_w, uint32_t src_h); > void (*disable_plane)(struct drm_plane *plane, > struct drm_crtc *crtc); > + int (*check_plane)(struct drm_plane *plane, > + struct intel_plane_state *state); > + void (*commit_plane)(struct drm_plane *plane, > + struct intel_plane_state *state); > int (*update_colorkey)(struct drm_plane *plane, > struct drm_intel_sprite_colorkey *key); > void (*get_colorkey)(struct drm_plane *plane, > @@ -536,6 +542,7 @@ struct cxsr_latency { > #define to_intel_encoder(x) container_of(x, struct intel_encoder, base) > #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base) > #define to_intel_plane(x) container_of(x, struct intel_plane, base) > +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base) > #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL) > > struct intel_hdmi { > @@ -1195,4 +1202,12 @@ int intel_sprite_plane_disable(struct drm_plane *plane); > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > > +/* intel_atomic.c */ > +struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); > +void intel_plane_destroy_state(struct drm_plane *plane, > + struct drm_plane_state *state); > +void intel_crtc_atomic_begin(struct drm_crtc *crtc); > +void intel_crtc_atomic_flush(struct drm_crtc *crtc); > +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > + > #endif /* __INTEL_DRV_H__ */ > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 9e6a72a..d3677c3 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -33,6 +33,7 @@ > #include <drm/drm_crtc.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_rect.h> > +#include <drm/drm_plane_helper.h> > #include "intel_drv.h" > #include <drm/i915_drm.h> > #include "i915_drv.h" > @@ -1573,6 +1574,8 @@ static const struct drm_plane_funcs intel_plane_funcs = { > .disable_plane = intel_sprite_plane_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > }; > > static uint32_t ilk_plane_formats[] = { > @@ -1634,6 +1637,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > if (!intel_plane) > return -ENOMEM; > > + intel_plane->base.state = > + intel_plane_duplicate_state(&intel_plane->base); > + > switch (INTEL_INFO(dev)->gen) { > case 5: > case 6: > @@ -1704,6 +1710,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > intel_plane->pipe = pipe; > intel_plane->plane = plane; > intel_plane->rotation = BIT(DRM_ROTATE_0); > + intel_plane->check_plane = intel_check_sprite_plane; > + intel_plane->commit_plane = intel_commit_sprite_plane; > possible_crtcs = (1 << pipe); > ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, > &intel_plane_funcs, > @@ -1725,6 +1733,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > dev->mode_config.rotation_property, > intel_plane->rotation); > > + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs); > + > out: > return ret; > } _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/7] drm/i915: Prepare for atomic plane helpers 2014-11-13 19:46 ` Bob Paauwe @ 2014-11-13 21:31 ` Matt Roper 0 siblings, 0 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 21:31 UTC (permalink / raw) To: Bob Paauwe; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 11:46:25AM -0800, Bob Paauwe wrote: > On Thu, 13 Nov 2014 10:43:24 -0800 > Matt Roper <matthew.d.roper@intel.com> wrote: ... > > + > > +/** > > + * intel_plane_destroy_state - destroy plane state > > + * @plane: drm plane > > + * > > + * Allocates and returns a copy of the plane state (both common and > > + * Intel-specific) for the specified plane. > > Comment should be updated for destroy. Good catch; will update. ... > > +static int intel_plane_atomic_check(struct drm_plane *plane, > > + struct drm_plane_state *state) > > +{ > > + struct intel_plane *intel_plane = to_intel_plane(plane); > > + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); > > + struct intel_plane_state *intel_state = to_intel_plane_state(state); > > + > > + /* Disabling a plane is always okay */ > > + if (state->fb == NULL) > > + return 0; > > + > > + /* > > + * The original src/dest coordinates are stored in state->base, but > > + * we want to keep another copy internal to our driver that we can > > + * clip/modify ourselves. > > + */ > > + intel_state->src.x1 = state->src_x; > > + intel_state->src.y1 = state->src_y; > > + intel_state->src.x2 = state->src_x + state->src_w; > > + intel_state->src.y2 = state->src_y + state->src_h; > > + intel_state->dst.x1 = state->crtc_x; > > + intel_state->dst.y1 = state->crtc_y; > > + intel_state->dst.x2 = state->crtc_x + state->crtc_w; > > + intel_state->dst.y2 = state->crtc_y + state->crtc_h; > > + > > + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ > > + if (intel_crtc) { > > + intel_state->clip.x1 = 0; > > + intel_state->clip.y1 = 0; > > + intel_state->clip.x2 = > > + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > > + intel_state->clip.y2 = > > + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > > Should this be using intel_crtc->new_config here? I'm not entierly > clear on how the crtc config/new_config would tie into the atomic state. I don't think so. config vs new_config tracks pipe configuration during the modeset sequence, but we're only doing a plane update here, so the current crtc configuration is what we'd clip against. I haven't converted over any of the CRTC modeset handling, just the plane updates. Matt -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) 2014-11-13 18:43 ` [PATCH 5/7] drm/i915: Prepare for atomic plane helpers Matt Roper 2014-11-13 19:46 ` Bob Paauwe @ 2014-11-13 22:51 ` Matt Roper 2014-11-14 1:23 ` Matt Roper 1 sibling, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-13 22:51 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Add the new driver entrypoints that will be called by the atomic plane helpers. This patch does not actually switch over to the new plane helpers yet, so there should be no functional change here. Also note that although plane programming was already split into check/prepare/commit steps, some of the semantics of those individual functions will need to change slightly when we do make the jump so that they match the behavior the plane helpers expect. v2: - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) - Fix a copy/paste comment mistake (Bob) Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 220 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_display.c | 20 ++- drivers/gpu/drm/i915/intel_drv.h | 15 ++ drivers/gpu/drm/i915/intel_sprite.c | 10 ++ 5 files changed, 265 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 891e584..9f5c57d 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -63,6 +63,7 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ + intel_atomic_plane.o \ intel_crt.o \ intel_ddi.o \ intel_dp.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c new file mode 100644 index 0000000..7c039bf --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -0,0 +1,220 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h> +#include <drm/drm_plane_helper.h> +#include "intel_drv.h" + +/** + * intel_plane_duplicate_state - duplicate plane state + * @plane: drm plane + * + * Allocates and returns a copy of the plane state (both common and + * Intel-specific) for the specified plane. + */ +struct drm_plane_state * +intel_plane_duplicate_state(struct drm_plane *plane) +{ + struct intel_plane_state *state; + + if (plane->state) + state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL); + else + state = kzalloc(sizeof(*state), GFP_KERNEL); + + if (state && state->base.fb) + drm_framebuffer_reference(state->base.fb); + + return &state->base; +} + +/** + * intel_plane_destroy_state - destroy plane state + * @plane: drm plane + * + * Destroys the plane state (both common and Intel-specific) for the + * specified plane. + */ +void +intel_plane_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + drm_atomic_helper_plane_destroy_state(plane, state); +} + + +/** + * intel_crtc_atomic_begin - Begins an atomic commit on a CRTC + * @crtc: drm crtc + * + * Prepares to write registers associated with the atomic commit of a CRTC + * by using vblank evasion to ensure that all register writes happen within + * the same vblank period. + */ +void intel_crtc_atomic_begin(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + intel_pipe_update_start(intel_crtc, &intel_crtc->atomic_vbl_count); +} + +/** + * intel_crtc_atomic_flush - Finishes an atomic commit on a CRTC + * @crtc: drm crtc + * + * Concludes the writing of registers for an atomic commit of a CRTC. + */ +void intel_crtc_atomic_flush(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + + intel_pipe_update_end(intel_crtc, intel_crtc->atomic_vbl_count); +} + +static int intel_prepare_fb(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = plane->dev; + struct intel_plane *intel_plane = to_intel_plane(plane); + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct drm_i915_gem_object *old_obj = intel_plane->obj; + enum pipe pipe = intel_plane->pipe; + unsigned front_bits = 0; + int ret = 0; + + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + front_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); + + if (plane->crtc) { + intel_crtc_wait_for_pending_flips(plane->crtc); + if (intel_crtc_has_pending_flip(plane->crtc)) { + DRM_ERROR("pipe is still busy with an old pageflip\n"); + return -EBUSY; + } + } + + break; + case DRM_PLANE_TYPE_OVERLAY: + front_bits = INTEL_FRONTBUFFER_SPRITE(pipe); + break; + case DRM_PLANE_TYPE_CURSOR: + front_bits = INTEL_FRONTBUFFER_CURSOR(pipe); + break; + } + + mutex_lock(&dev->struct_mutex); + + /* Note that this will apply the VT-d workaround for scanouts, + * which is more restrictive than required for sprites. (The + * primary plane requires 256KiB alignment with 64 PTE padding, + * the sprite planes only require 128KiB alignment and 32 PTE + * padding. + */ + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); + if (ret == 0) + i915_gem_track_fb(old_obj, obj, front_bits); + + if (plane->type == DRM_PLANE_TYPE_CURSOR && + INTEL_INFO(dev)->cursor_needs_physical) { + int align = IS_I830(dev) ? 16 * 1024 : 256; + + ret = i915_gem_object_attach_phys(obj, align); + if (ret) + DRM_DEBUG_KMS("failed to attach phys object\n"); + } + + mutex_unlock(&dev->struct_mutex); + + return ret; +} + +static void intel_cleanup_fb(struct drm_plane *plane, + struct drm_framebuffer *fb) +{ + struct drm_device *dev = plane->dev; + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + + mutex_lock(&dev->struct_mutex); + intel_unpin_fb_obj(obj); + mutex_unlock(&dev->struct_mutex); +} + +static int intel_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); + struct intel_plane_state *intel_state = to_intel_plane_state(state); + + /* Disabling a plane is always okay */ + if (state->fb == NULL) + return 0; + + /* + * The original src/dest coordinates are stored in state->base, but + * we want to keep another copy internal to our driver that we can + * clip/modify ourselves. + */ + intel_state->src.x1 = state->src_x; + intel_state->src.y1 = state->src_y; + intel_state->src.x2 = state->src_x + state->src_w; + intel_state->src.y2 = state->src_y + state->src_h; + intel_state->dst.x1 = state->crtc_x; + intel_state->dst.y1 = state->crtc_y; + intel_state->dst.x2 = state->crtc_x + state->crtc_w; + intel_state->dst.y2 = state->crtc_y + state->crtc_h; + + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ + if (intel_crtc) { + intel_state->clip.x1 = 0; + intel_state->clip.y1 = 0; + intel_state->clip.x2 = + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; + intel_state->clip.y2 = + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; + } + + return intel_plane->check_plane(plane, intel_state); +} + +static void intel_plane_atomic_update(struct drm_plane *plane) +{ + struct intel_plane *intel_plane = to_intel_plane(plane); + struct intel_plane_state *intel_state = + to_intel_plane_state(plane->state); + + if (!plane->state->fb) + intel_plane_disable(plane); + else + intel_plane->commit_plane(plane, intel_state); +} + +const struct drm_plane_helper_funcs intel_plane_helper_funcs = { + .prepare_fb = intel_prepare_fb, + .cleanup_fb = intel_cleanup_fb, + .atomic_check = intel_plane_atomic_check, + .atomic_update = intel_plane_atomic_update, +}; + diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2d4ef04..32a2b12 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9705,6 +9705,8 @@ out_hang: static struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, .load_lut = intel_crtc_load_lut, + .atomic_begin = intel_crtc_atomic_begin, + .atomic_flush = intel_crtc_atomic_flush, }; /** @@ -11747,6 +11749,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, void intel_plane_destroy(struct drm_plane *plane) { struct intel_plane *intel_plane = to_intel_plane(plane); + intel_plane_destroy_state(plane, plane->state); drm_plane_cleanup(plane); kfree(intel_plane); } @@ -11755,7 +11758,10 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = { .update_plane = intel_primary_plane_setplane, .disable_plane = intel_primary_plane_disable, .destroy = intel_plane_destroy, - .set_property = intel_plane_set_property + .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, + }; static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, @@ -11769,11 +11775,14 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, if (primary == NULL) return NULL; + primary->base.state = intel_plane_duplicate_state(&primary->base); primary->can_scale = false; primary->max_downscale = 1; primary->pipe = pipe; primary->plane = pipe; primary->rotation = BIT(DRM_ROTATE_0); + primary->check_plane = intel_check_primary_plane; + primary->commit_plane = intel_commit_primary_plane; if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) primary->plane = !pipe; @@ -11802,6 +11811,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, primary->rotation); } + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs); + return &primary->base; } @@ -11957,6 +11968,8 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { .disable_plane = intel_cursor_plane_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, }; static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, @@ -11968,11 +11981,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, if (cursor == NULL) return NULL; + cursor->base.state = intel_plane_duplicate_state(&cursor->base); cursor->can_scale = false; cursor->max_downscale = 1; cursor->pipe = pipe; cursor->plane = pipe; cursor->rotation = BIT(DRM_ROTATE_0); + cursor->check_plane = intel_check_cursor_plane; + cursor->commit_plane = intel_commit_cursor_plane; drm_universal_plane_init(dev, &cursor->base, 0, &intel_cursor_plane_funcs, @@ -11992,6 +12008,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, cursor->rotation); } + drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs); + return &cursor->base; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 49358c8..1882de1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -466,6 +466,8 @@ struct intel_crtc { int scanline_offset; struct intel_mmio_flip mmio_flip; + + uint32_t atomic_vbl_count; }; struct intel_plane_wm_parameters { @@ -506,6 +508,10 @@ struct intel_plane { uint32_t src_w, uint32_t src_h); void (*disable_plane)(struct drm_plane *plane, struct drm_crtc *crtc); + int (*check_plane)(struct drm_plane *plane, + struct intel_plane_state *state); + void (*commit_plane)(struct drm_plane *plane, + struct intel_plane_state *state); int (*update_colorkey)(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key); void (*get_colorkey)(struct drm_plane *plane, @@ -536,6 +542,7 @@ struct cxsr_latency { #define to_intel_encoder(x) container_of(x, struct intel_encoder, base) #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base) #define to_intel_plane(x) container_of(x, struct intel_plane, base) +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base) #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL) struct intel_hdmi { @@ -1195,4 +1202,12 @@ int intel_sprite_plane_disable(struct drm_plane *plane); /* intel_tv.c */ void intel_tv_init(struct drm_device *dev); +/* intel_atomic.c */ +struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); +void intel_plane_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state); +void intel_crtc_atomic_begin(struct drm_crtc *crtc); +void intel_crtc_atomic_flush(struct drm_crtc *crtc); +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; + #endif /* __INTEL_DRV_H__ */ diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 9e6a72a..d3677c3 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -33,6 +33,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> #include <drm/drm_rect.h> +#include <drm/drm_plane_helper.h> #include "intel_drv.h" #include <drm/i915_drm.h> #include "i915_drv.h" @@ -1573,6 +1574,8 @@ static const struct drm_plane_funcs intel_plane_funcs = { .disable_plane = intel_sprite_plane_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, + .atomic_duplicate_state = intel_plane_duplicate_state, + .atomic_destroy_state = intel_plane_destroy_state, }; static uint32_t ilk_plane_formats[] = { @@ -1634,6 +1637,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) if (!intel_plane) return -ENOMEM; + intel_plane->base.state = + intel_plane_duplicate_state(&intel_plane->base); + switch (INTEL_INFO(dev)->gen) { case 5: case 6: @@ -1704,6 +1710,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) intel_plane->pipe = pipe; intel_plane->plane = plane; intel_plane->rotation = BIT(DRM_ROTATE_0); + intel_plane->check_plane = intel_check_sprite_plane; + intel_plane->commit_plane = intel_commit_sprite_plane; possible_crtcs = (1 << pipe); ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, &intel_plane_funcs, @@ -1725,6 +1733,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) dev->mode_config.rotation_property, intel_plane->rotation); + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs); + out: return ret; } -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) 2014-11-13 22:51 ` [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) Matt Roper @ 2014-11-14 1:23 ` Matt Roper 2014-11-14 7:48 ` Daniel Vetter 0 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-11-14 1:23 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel On Thu, Nov 13, 2014 at 02:51:32PM -0800, Matt Roper wrote: > Add the new driver entrypoints that will be called by the atomic plane > helpers. > > This patch does not actually switch over to the new plane helpers yet, > so there should be no functional change here. Also note that although > plane programming was already split into check/prepare/commit steps, > some of the semantics of those individual functions will need to change > slightly when we do make the jump so that they match the behavior the > plane helpers expect. > > v2: > - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) > - Fix a copy/paste comment mistake (Bob) > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Actually, I don't think this is quite ready to go yet. I must have had too much debug spam turned on before and missed the important messages in the kernel log, but it looks like we're still calling something that can sleep (in the frontbuffer tracking code?) while irqs are disabled for vblank evasion. I'll have to look into this some more tomorrow and decide what an appropriate fix is. Matt > --- > drivers/gpu/drm/i915/Makefile | 1 + > drivers/gpu/drm/i915/intel_atomic_plane.c | 220 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 20 ++- > drivers/gpu/drm/i915/intel_drv.h | 15 ++ > drivers/gpu/drm/i915/intel_sprite.c | 10 ++ > 5 files changed, 265 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 891e584..9f5c57d 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -63,6 +63,7 @@ i915-y += dvo_ch7017.o \ > dvo_ns2501.o \ > dvo_sil164.o \ > dvo_tfp410.o \ > + intel_atomic_plane.o \ > intel_crt.o \ > intel_ddi.o \ > intel_dp.o \ > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c > new file mode 100644 > index 0000000..7c039bf > --- /dev/null > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c > @@ -0,0 +1,220 @@ > +/* > + * Copyright © 2014 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_atomic_helper.h> > +#include <drm/drm_plane_helper.h> > +#include "intel_drv.h" > + > +/** > + * intel_plane_duplicate_state - duplicate plane state > + * @plane: drm plane > + * > + * Allocates and returns a copy of the plane state (both common and > + * Intel-specific) for the specified plane. > + */ > +struct drm_plane_state * > +intel_plane_duplicate_state(struct drm_plane *plane) > +{ > + struct intel_plane_state *state; > + > + if (plane->state) > + state = kmemdup(plane->state, sizeof(*state), GFP_KERNEL); > + else > + state = kzalloc(sizeof(*state), GFP_KERNEL); > + > + if (state && state->base.fb) > + drm_framebuffer_reference(state->base.fb); > + > + return &state->base; > +} > + > +/** > + * intel_plane_destroy_state - destroy plane state > + * @plane: drm plane > + * > + * Destroys the plane state (both common and Intel-specific) for the > + * specified plane. > + */ > +void > +intel_plane_destroy_state(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + drm_atomic_helper_plane_destroy_state(plane, state); > +} > + > + > +/** > + * intel_crtc_atomic_begin - Begins an atomic commit on a CRTC > + * @crtc: drm crtc > + * > + * Prepares to write registers associated with the atomic commit of a CRTC > + * by using vblank evasion to ensure that all register writes happen within > + * the same vblank period. > + */ > +void intel_crtc_atomic_begin(struct drm_crtc *crtc) > +{ > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + > + intel_pipe_update_start(intel_crtc, &intel_crtc->atomic_vbl_count); > +} > + > +/** > + * intel_crtc_atomic_flush - Finishes an atomic commit on a CRTC > + * @crtc: drm crtc > + * > + * Concludes the writing of registers for an atomic commit of a CRTC. > + */ > +void intel_crtc_atomic_flush(struct drm_crtc *crtc) > +{ > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + > + intel_pipe_update_end(intel_crtc, intel_crtc->atomic_vbl_count); > +} > + > +static int intel_prepare_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct drm_i915_gem_object *old_obj = intel_plane->obj; > + enum pipe pipe = intel_plane->pipe; > + unsigned front_bits = 0; > + int ret = 0; > + > + switch (plane->type) { > + case DRM_PLANE_TYPE_PRIMARY: > + front_bits = INTEL_FRONTBUFFER_PRIMARY(pipe); > + > + if (plane->crtc) { > + intel_crtc_wait_for_pending_flips(plane->crtc); > + if (intel_crtc_has_pending_flip(plane->crtc)) { > + DRM_ERROR("pipe is still busy with an old pageflip\n"); > + return -EBUSY; > + } > + } > + > + break; > + case DRM_PLANE_TYPE_OVERLAY: > + front_bits = INTEL_FRONTBUFFER_SPRITE(pipe); > + break; > + case DRM_PLANE_TYPE_CURSOR: > + front_bits = INTEL_FRONTBUFFER_CURSOR(pipe); > + break; > + } > + > + mutex_lock(&dev->struct_mutex); > + > + /* Note that this will apply the VT-d workaround for scanouts, > + * which is more restrictive than required for sprites. (The > + * primary plane requires 256KiB alignment with 64 PTE padding, > + * the sprite planes only require 128KiB alignment and 32 PTE > + * padding. > + */ > + ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > + if (ret == 0) > + i915_gem_track_fb(old_obj, obj, front_bits); > + > + if (plane->type == DRM_PLANE_TYPE_CURSOR && > + INTEL_INFO(dev)->cursor_needs_physical) { > + int align = IS_I830(dev) ? 16 * 1024 : 256; > + > + ret = i915_gem_object_attach_phys(obj, align); > + if (ret) > + DRM_DEBUG_KMS("failed to attach phys object\n"); > + } > + > + mutex_unlock(&dev->struct_mutex); > + > + return ret; > +} > + > +static void intel_cleanup_fb(struct drm_plane *plane, > + struct drm_framebuffer *fb) > +{ > + struct drm_device *dev = plane->dev; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + > + mutex_lock(&dev->struct_mutex); > + intel_unpin_fb_obj(obj); > + mutex_unlock(&dev->struct_mutex); > +} > + > +static int intel_plane_atomic_check(struct drm_plane *plane, > + struct drm_plane_state *state) > +{ > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc); > + struct intel_plane_state *intel_state = to_intel_plane_state(state); > + > + /* Disabling a plane is always okay */ > + if (state->fb == NULL) > + return 0; > + > + /* > + * The original src/dest coordinates are stored in state->base, but > + * we want to keep another copy internal to our driver that we can > + * clip/modify ourselves. > + */ > + intel_state->src.x1 = state->src_x; > + intel_state->src.y1 = state->src_y; > + intel_state->src.x2 = state->src_x + state->src_w; > + intel_state->src.y2 = state->src_y + state->src_h; > + intel_state->dst.x1 = state->crtc_x; > + intel_state->dst.y1 = state->crtc_y; > + intel_state->dst.x2 = state->crtc_x + state->crtc_w; > + intel_state->dst.y2 = state->crtc_y + state->crtc_h; > + > + /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */ > + if (intel_crtc) { > + intel_state->clip.x1 = 0; > + intel_state->clip.y1 = 0; > + intel_state->clip.x2 = > + intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > + intel_state->clip.y2 = > + intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > + } > + > + return intel_plane->check_plane(plane, intel_state); > +} > + > +static void intel_plane_atomic_update(struct drm_plane *plane) > +{ > + struct intel_plane *intel_plane = to_intel_plane(plane); > + struct intel_plane_state *intel_state = > + to_intel_plane_state(plane->state); > + > + if (!plane->state->fb) > + intel_plane_disable(plane); > + else > + intel_plane->commit_plane(plane, intel_state); > +} > + > +const struct drm_plane_helper_funcs intel_plane_helper_funcs = { > + .prepare_fb = intel_prepare_fb, > + .cleanup_fb = intel_cleanup_fb, > + .atomic_check = intel_plane_atomic_check, > + .atomic_update = intel_plane_atomic_update, > +}; > + > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2d4ef04..32a2b12 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9705,6 +9705,8 @@ out_hang: > static struct drm_crtc_helper_funcs intel_helper_funcs = { > .mode_set_base_atomic = intel_pipe_set_base_atomic, > .load_lut = intel_crtc_load_lut, > + .atomic_begin = intel_crtc_atomic_begin, > + .atomic_flush = intel_crtc_atomic_flush, > }; > > /** > @@ -11747,6 +11749,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > void intel_plane_destroy(struct drm_plane *plane) > { > struct intel_plane *intel_plane = to_intel_plane(plane); > + intel_plane_destroy_state(plane, plane->state); > drm_plane_cleanup(plane); > kfree(intel_plane); > } > @@ -11755,7 +11758,10 @@ static const struct drm_plane_funcs intel_primary_plane_funcs = { > .update_plane = intel_primary_plane_setplane, > .disable_plane = intel_primary_plane_disable, > .destroy = intel_plane_destroy, > - .set_property = intel_plane_set_property > + .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > + > }; > > static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > @@ -11769,11 +11775,14 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > if (primary == NULL) > return NULL; > > + primary->base.state = intel_plane_duplicate_state(&primary->base); > primary->can_scale = false; > primary->max_downscale = 1; > primary->pipe = pipe; > primary->plane = pipe; > primary->rotation = BIT(DRM_ROTATE_0); > + primary->check_plane = intel_check_primary_plane; > + primary->commit_plane = intel_commit_primary_plane; > if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) > primary->plane = !pipe; > > @@ -11802,6 +11811,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > primary->rotation); > } > > + drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs); > + > return &primary->base; > } > > @@ -11957,6 +11968,8 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = { > .disable_plane = intel_cursor_plane_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > }; > > static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > @@ -11968,11 +11981,14 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > if (cursor == NULL) > return NULL; > > + cursor->base.state = intel_plane_duplicate_state(&cursor->base); > cursor->can_scale = false; > cursor->max_downscale = 1; > cursor->pipe = pipe; > cursor->plane = pipe; > cursor->rotation = BIT(DRM_ROTATE_0); > + cursor->check_plane = intel_check_cursor_plane; > + cursor->commit_plane = intel_commit_cursor_plane; > > drm_universal_plane_init(dev, &cursor->base, 0, > &intel_cursor_plane_funcs, > @@ -11992,6 +12008,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev, > cursor->rotation); > } > > + drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs); > + > return &cursor->base; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 49358c8..1882de1 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -466,6 +466,8 @@ struct intel_crtc { > > int scanline_offset; > struct intel_mmio_flip mmio_flip; > + > + uint32_t atomic_vbl_count; > }; > > struct intel_plane_wm_parameters { > @@ -506,6 +508,10 @@ struct intel_plane { > uint32_t src_w, uint32_t src_h); > void (*disable_plane)(struct drm_plane *plane, > struct drm_crtc *crtc); > + int (*check_plane)(struct drm_plane *plane, > + struct intel_plane_state *state); > + void (*commit_plane)(struct drm_plane *plane, > + struct intel_plane_state *state); > int (*update_colorkey)(struct drm_plane *plane, > struct drm_intel_sprite_colorkey *key); > void (*get_colorkey)(struct drm_plane *plane, > @@ -536,6 +542,7 @@ struct cxsr_latency { > #define to_intel_encoder(x) container_of(x, struct intel_encoder, base) > #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer, base) > #define to_intel_plane(x) container_of(x, struct intel_plane, base) > +#define to_intel_plane_state(x) container_of(x, struct intel_plane_state, base) > #define intel_fb_obj(x) (x ? to_intel_framebuffer(x)->obj : NULL) > > struct intel_hdmi { > @@ -1195,4 +1202,12 @@ int intel_sprite_plane_disable(struct drm_plane *plane); > /* intel_tv.c */ > void intel_tv_init(struct drm_device *dev); > > +/* intel_atomic.c */ > +struct drm_plane_state *intel_plane_duplicate_state(struct drm_plane *plane); > +void intel_plane_destroy_state(struct drm_plane *plane, > + struct drm_plane_state *state); > +void intel_crtc_atomic_begin(struct drm_crtc *crtc); > +void intel_crtc_atomic_flush(struct drm_crtc *crtc); > +extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > + > #endif /* __INTEL_DRV_H__ */ > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 9e6a72a..d3677c3 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -33,6 +33,7 @@ > #include <drm/drm_crtc.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_rect.h> > +#include <drm/drm_plane_helper.h> > #include "intel_drv.h" > #include <drm/i915_drm.h> > #include "i915_drv.h" > @@ -1573,6 +1574,8 @@ static const struct drm_plane_funcs intel_plane_funcs = { > .disable_plane = intel_sprite_plane_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > + .atomic_duplicate_state = intel_plane_duplicate_state, > + .atomic_destroy_state = intel_plane_destroy_state, > }; > > static uint32_t ilk_plane_formats[] = { > @@ -1634,6 +1637,9 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > if (!intel_plane) > return -ENOMEM; > > + intel_plane->base.state = > + intel_plane_duplicate_state(&intel_plane->base); > + > switch (INTEL_INFO(dev)->gen) { > case 5: > case 6: > @@ -1704,6 +1710,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > intel_plane->pipe = pipe; > intel_plane->plane = plane; > intel_plane->rotation = BIT(DRM_ROTATE_0); > + intel_plane->check_plane = intel_check_sprite_plane; > + intel_plane->commit_plane = intel_commit_sprite_plane; > possible_crtcs = (1 << pipe); > ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, > &intel_plane_funcs, > @@ -1725,6 +1733,8 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) > dev->mode_config.rotation_property, > intel_plane->rotation); > > + drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs); > + > out: > return ret; > } > -- > 1.8.5.1 > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) 2014-11-14 1:23 ` Matt Roper @ 2014-11-14 7:48 ` Daniel Vetter 0 siblings, 0 replies; 29+ messages in thread From: Daniel Vetter @ 2014-11-14 7:48 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 05:23:05PM -0800, Matt Roper wrote: > On Thu, Nov 13, 2014 at 02:51:32PM -0800, Matt Roper wrote: > > Add the new driver entrypoints that will be called by the atomic plane > > helpers. > > > > This patch does not actually switch over to the new plane helpers yet, > > so there should be no functional change here. Also note that although > > plane programming was already split into check/prepare/commit steps, > > some of the semantics of those individual functions will need to change > > slightly when we do make the jump so that they match the behavior the > > plane helpers expect. > > > > v2: > > - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) > > - Fix a copy/paste comment mistake (Bob) > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > Actually, I don't think this is quite ready to go yet. I must have had > too much debug spam turned on before and missed the important messages > in the kernel log, but it looks like we're still calling something that > can sleep (in the frontbuffer tracking code?) while irqs are disabled > for vblank evasion. Yeah the frontbuffer tracking code will sleep since it can grab mutexes. We also want to eventually grow async support out of this, so I think the best approach might be to simply use frontbuffer_flip_prepare/complete around the entire sequence. Well the important part is that we call prepare before we start waiting for outstanding rendering and complete after pipe_update_end. Since this loops over abstract plane objects I think we need to store the correct frontbuffer tracking bit somewhere in intel_plane so that we can get at them. -Daniel -- 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] 29+ messages in thread
* [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper ` (4 preceding siblings ...) 2014-11-13 18:43 ` [PATCH 5/7] drm/i915: Prepare for atomic plane helpers Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 21:28 ` Bob Paauwe ` (2 more replies) 2014-11-13 18:43 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper 2014-11-13 22:53 ` [PATCH 8/8] drm/i915: Integrate kerneldoc for atomic plane helpers Matt Roper 7 siblings, 3 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Now that we have hooks to enable the atomic plane helpers, we can use the plane helpers for our .update_plane() and .disable_plane() entrypoints. Even though we'd already refactored our plane handling code into check/commit, we still have to rework some of that logic here as we make the transition. The atomic plane helpers now call prepare_fb/cleanup_fb for us, so note the following: - 'commit' should not longer unpin the old object; the cleanup_fb will take care of this for us - prepare_fb only gets called when we actually have an fb, so the internal 'disable' code needs to update frontbuffer tracking - the cursor plane code never had 'prepare' code separated from its 'commit' function, so a lot of the cursor's 'commit' needs to be dropped since prepare_fb already does it for us - with all the pinning work removed from cursor commit, it can no longer fail and should return void rather than int Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 255 +++++------------------------------ drivers/gpu/drm/i915/intel_sprite.c | 122 ++--------------- 2 files changed, 41 insertions(+), 336 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 32a2b12..8d5a3b4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8285,17 +8285,15 @@ static bool cursor_size_ok(struct drm_device *dev, return true; } -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, - struct drm_i915_gem_object *obj, - uint32_t width, uint32_t height) +static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc, + struct drm_i915_gem_object *obj, + uint32_t width, uint32_t height) { struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; unsigned old_width; uint32_t addr; - int ret; /* if we want to turn off the cursor ignore width and height */ if (!obj) { @@ -8305,64 +8303,14 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, goto finish; } - /* we only need to pin inside GTT if cursor is non-phy */ + /* plane helper's prepare_fb() already handled pinning */ mutex_lock(&dev->struct_mutex); - if (!INTEL_INFO(dev)->cursor_needs_physical) { - unsigned alignment; - - /* - * Global gtt pte registers are special registers which actually - * forward writes to a chunk of system memory. Which means that - * there is no risk that the register values disappear as soon - * as we call intel_runtime_pm_put(), so it is correct to wrap - * only the pin/unpin/fence and not more. - */ - intel_runtime_pm_get(dev_priv); - - /* Note that the w/a also requires 2 PTE of padding following - * the bo. We currently fill all unused PTE with the shadow - * page and so we should always have valid PTE following the - * cursor preventing the VT-d warning. - */ - alignment = 0; - if (need_vtd_wa(dev)) - alignment = 64*1024; - - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); - if (ret) { - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); - intel_runtime_pm_put(dev_priv); - goto fail_locked; - } - - ret = i915_gem_object_put_fence(obj); - if (ret) { - DRM_DEBUG_KMS("failed to release fence for cursor"); - intel_runtime_pm_put(dev_priv); - goto fail_unpin; - } - + if (!INTEL_INFO(dev)->cursor_needs_physical) addr = i915_gem_obj_ggtt_offset(obj); - - intel_runtime_pm_put(dev_priv); - } else { - int align = IS_I830(dev) ? 16 * 1024 : 256; - ret = i915_gem_object_attach_phys(obj, align); - if (ret) { - DRM_DEBUG_KMS("failed to attach phys object\n"); - goto fail_locked; - } + else addr = obj->phys_handle->busaddr; - } finish: - if (intel_crtc->cursor_bo) { - if (!INTEL_INFO(dev)->cursor_needs_physical) - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); - } - - i915_gem_track_fb(intel_crtc->cursor_bo, obj, - INTEL_FRONTBUFFER_CURSOR(pipe)); mutex_unlock(&dev->struct_mutex); old_width = intel_crtc->cursor_width; @@ -8379,13 +8327,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); } - - return 0; -fail_unpin: - i915_gem_object_unpin_from_display_plane(obj); -fail_locked: - mutex_unlock(&dev->struct_mutex); - return ret; } static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, @@ -11544,7 +11485,6 @@ disable_unpin: mutex_lock(&dev->struct_mutex); i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); - intel_unpin_fb_obj(intel_fb_obj(plane->fb)); mutex_unlock(&dev->struct_mutex); plane->fb = NULL; @@ -11568,42 +11508,6 @@ intel_check_primary_plane(struct drm_plane *plane, false, true, &state->visible); } -static int -intel_prepare_primary_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_crtc *crtc = state->base.crtc; - struct drm_framebuffer *fb = state->base.fb; - struct drm_device *dev = crtc->dev; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); - int ret; - - intel_crtc_wait_for_pending_flips(crtc); - - if (intel_crtc_has_pending_flip(crtc)) { - DRM_ERROR("pipe is still busy with an old pageflip\n"); - return -EBUSY; - } - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret != 0) { - DRM_DEBUG_KMS("pin & fence failed\n"); - return ret; - } - } - - return 0; -} - static void intel_commit_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -11614,9 +11518,7 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *old_fb = plane->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_rect *src = &state->src; @@ -11687,62 +11589,6 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_update_fbc(dev); mutex_unlock(&dev->struct_mutex); } - - if (old_fb && old_fb != fb) { - if (intel_crtc->active) - intel_wait_for_vblank(dev, intel_crtc->pipe); - - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } -} - -static int -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_plane_state state; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_primary_plane(plane, &state); - if (ret) - return ret; - - ret = intel_prepare_primary_plane(plane, &state); - if (ret) - return ret; - - intel_commit_primary_plane(plane, &state); - - return 0; } /* Common destruction function for both primary and cursor planes */ @@ -11755,8 +11601,8 @@ void intel_plane_destroy(struct drm_plane *plane) } static const struct drm_plane_funcs intel_primary_plane_funcs = { - .update_plane = intel_primary_plane_setplane, - .disable_plane = intel_primary_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, @@ -11816,15 +11662,22 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, return &primary->base; } -static int +static void intel_cursor_plane_disable(struct drm_plane *plane) { + struct intel_plane *intel_plane = to_intel_plane(plane); + if (!plane->fb) - return 0; + return; BUG_ON(!plane->crtc); - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); + intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); + + mutex_lock(&plane->dev->struct_mutex); + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, + INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe)); + mutex_unlock(&plane->dev->struct_mutex); } static int @@ -11838,7 +11691,6 @@ intel_check_cursor_plane(struct drm_plane *plane, struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - int crtc_w, crtc_h; unsigned stride; int ret; @@ -11856,15 +11708,14 @@ intel_check_cursor_plane(struct drm_plane *plane, return 0; /* Check for which cursor types we support */ - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { - DRM_DEBUG("Cursor dimension not supported\n"); + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { + DRM_DEBUG("Cursor dimension %dx%d not supported\n", + state->base.crtc_w, state->base.crtc_h); return -EINVAL; } - stride = roundup_pow_of_two(crtc_w) * 4; - if (obj->base.size < stride * crtc_h) { + stride = roundup_pow_of_two(state->base.crtc_w) * 4; + if (obj->base.size < stride * state->base.crtc_h) { DRM_DEBUG_KMS("buffer is too small\n"); return -ENOMEM; } @@ -11883,7 +11734,7 @@ intel_check_cursor_plane(struct drm_plane *plane, return ret; } -static int +static void intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { @@ -11893,10 +11744,9 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; - int crtc_w, crtc_h; - crtc->cursor_x = state->orig_dst.x1; - crtc->cursor_y = state->orig_dst.y1; + crtc->cursor_x = state->base.crtc_x; + crtc->cursor_y = state->base.crtc_y; intel_plane->crtc_x = state->orig_dst.x1; intel_plane->crtc_y = state->orig_dst.y1; @@ -11909,63 +11759,20 @@ intel_commit_cursor_plane(struct drm_plane *plane, intel_plane->obj = obj; if (fb != crtc->cursor->fb) { - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); + intel_crtc_cursor_set_obj(crtc, obj, + state->base.crtc_w, + state->base.crtc_h); } else { intel_crtc_update_cursor(crtc, state->visible); intel_frontbuffer_flip(crtc->dev, INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); - - return 0; } } -static int -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_plane_state state; - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_cursor_plane(plane, &state); - if (ret) - return ret; - - return intel_commit_cursor_plane(plane, &state); -} - static const struct drm_plane_funcs intel_cursor_plane_funcs = { - .update_plane = intel_cursor_plane_update, - .disable_plane = intel_cursor_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index d3677c3..28ccaf7 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1106,7 +1106,6 @@ intel_check_sprite_plane(struct drm_plane *plane, uint32_t src_x, src_y, src_w, src_h; struct drm_rect *src = &state->src; struct drm_rect *dst = &state->dst; - struct drm_rect *orig_src = &state->orig_src; const struct drm_rect *clip = &state->clip; int hscale, vscale; int max_scale, min_scale; @@ -1187,10 +1186,10 @@ intel_check_sprite_plane(struct drm_plane *plane, intel_plane->rotation); /* sanity check to make sure the src viewport wasn't enlarged */ - WARN_ON(src->x1 < (int) orig_src->x1 || - src->y1 < (int) orig_src->y1 || - src->x2 > (int) orig_src->x2 || - src->y2 > (int) orig_src->y2); + WARN_ON(src->x1 < (int) state->base.src_x || + src->y1 < (int) state->base.src_y || + src->x2 > (int) state->base.src_x + state->base.src_w || + src->y2 > (int) state->base.src_y + state->base.src_h); /* * Hardware doesn't handle subpixel coordinates. @@ -1258,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, return 0; } -static int -intel_prepare_sprite_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_device *dev = plane->dev; - struct drm_crtc *crtc = state->base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_plane *intel_plane = to_intel_plane(plane); - enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *fb = state->base.fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_plane->obj; - int ret; - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - - /* Note that this will apply the VT-d workaround for scanouts, - * which is more restrictive than required for sprites. (The - * primary plane requires 256KiB alignment with 64 PTE padding, - * the sprite planes only require 128KiB alignment and 32 PTE - * padding. - */ - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_SPRITE(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret) - return ret; - } - - return 0; -} - static void intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -1304,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, enum pipe pipe = intel_crtc->pipe; struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_plane->obj; int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; @@ -1362,68 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, if (!primary_was_enabled && primary_enabled) intel_post_enable_primary(crtc); } - - /* Unpin old obj after new one is active to avoid ugliness */ - if (old_obj && old_obj != obj) { - - /* - * It's fairly common to simply update the position of - * an existing object. In that case, we don't need to - * wait for vblank to avoid ugliness, we only need to - * do the pin & ref bookkeeping. - */ - if (intel_crtc->active) - intel_wait_for_vblank(dev, intel_crtc->pipe); - - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } -} - -static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_plane_state state; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_sprite_plane(plane, &state); - if (ret) - return ret; - - ret = intel_prepare_sprite_plane(plane, &state); - if (ret) - return ret; - - intel_commit_sprite_plane(plane, &state); - return 0; } int @@ -1459,7 +1360,6 @@ intel_sprite_plane_disable(struct drm_plane *plane) intel_wait_for_vblank(dev, intel_plane->pipe); mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(intel_plane->obj); i915_gem_track_fb(intel_plane->obj, NULL, INTEL_FRONTBUFFER_SPRITE(pipe)); mutex_unlock(&dev->struct_mutex); @@ -1557,21 +1457,19 @@ int intel_plane_set_property(struct drm_plane *plane, int intel_plane_restore(struct drm_plane *plane) { - struct intel_plane *intel_plane = to_intel_plane(plane); - if (!plane->crtc || !plane->fb) return 0; return plane->funcs->update_plane(plane, plane->crtc, plane->fb, - intel_plane->crtc_x, intel_plane->crtc_y, - intel_plane->crtc_w, intel_plane->crtc_h, - intel_plane->src_x, intel_plane->src_y, - intel_plane->src_w, intel_plane->src_h); + plane->state->crtc_x, plane->state->crtc_y, + plane->state->crtc_w, plane->state->crtc_h, + plane->state->src_x, plane->state->src_y, + plane->state->src_w, plane->state->src_h); } static const struct drm_plane_funcs intel_plane_funcs = { - .update_plane = intel_update_plane, - .disable_plane = intel_sprite_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, -- 1.8.5.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper @ 2014-11-13 21:28 ` Bob Paauwe 2014-11-13 22:52 ` [PATCH 6/8] drm/i915: Switch plane handling to atomic helpers (v2) Matt Roper 2014-11-14 9:39 ` [Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Ville Syrjälä 2 siblings, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 21:28 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:25 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > Now that we have hooks to enable the atomic plane helpers, we can use > the plane helpers for our .update_plane() and .disable_plane() > entrypoints. > > Even though we'd already refactored our plane handling code into > check/commit, we still have to rework some of that logic here as we make > the transition. The atomic plane helpers now call prepare_fb/cleanup_fb > for us, so note the following: > - 'commit' should not longer unpin the old object; the cleanup_fb will s/not/no/ > take care of this for us > - prepare_fb only gets called when we actually have an fb, so the > internal 'disable' code needs to update frontbuffer tracking > - the cursor plane code never had 'prepare' code separated from its > 'commit' function, so a lot of the cursor's 'commit' needs to be > dropped since prepare_fb already does it for us > - with all the pinning work removed from cursor commit, it can no > longer fail and should return void rather than int > > Testcase: igt/kms_plane > Testcase: igt/kms_universal_plane > Testcase: igt/kms_cursor_crc > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> With the above typo fixed this is Acknowledged-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 255 +++++------------------------------ > drivers/gpu/drm/i915/intel_sprite.c | 122 ++--------------- > 2 files changed, 41 insertions(+), 336 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 32a2b12..8d5a3b4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8285,17 +8285,15 @@ static bool cursor_size_ok(struct drm_device *dev, > return true; > } > > -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > - struct drm_i915_gem_object *obj, > - uint32_t width, uint32_t height) > +static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > + struct drm_i915_gem_object *obj, > + uint32_t width, uint32_t height) > { > struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum pipe pipe = intel_crtc->pipe; > unsigned old_width; > uint32_t addr; > - int ret; > > /* if we want to turn off the cursor ignore width and height */ > if (!obj) { > @@ -8305,64 +8303,14 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > goto finish; > } > > - /* we only need to pin inside GTT if cursor is non-phy */ > + /* plane helper's prepare_fb() already handled pinning */ > mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > - unsigned alignment; > - > - /* > - * Global gtt pte registers are special registers which actually > - * forward writes to a chunk of system memory. Which means that > - * there is no risk that the register values disappear as soon > - * as we call intel_runtime_pm_put(), so it is correct to wrap > - * only the pin/unpin/fence and not more. > - */ > - intel_runtime_pm_get(dev_priv); > - > - /* Note that the w/a also requires 2 PTE of padding following > - * the bo. We currently fill all unused PTE with the shadow > - * page and so we should always have valid PTE following the > - * cursor preventing the VT-d warning. > - */ > - alignment = 0; > - if (need_vtd_wa(dev)) > - alignment = 64*1024; > - > - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > - if (ret) { > - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > - intel_runtime_pm_put(dev_priv); > - goto fail_locked; > - } > - > - ret = i915_gem_object_put_fence(obj); > - if (ret) { > - DRM_DEBUG_KMS("failed to release fence for cursor"); > - intel_runtime_pm_put(dev_priv); > - goto fail_unpin; > - } > - > + if (!INTEL_INFO(dev)->cursor_needs_physical) > addr = i915_gem_obj_ggtt_offset(obj); > - > - intel_runtime_pm_put(dev_priv); > - } else { > - int align = IS_I830(dev) ? 16 * 1024 : 256; > - ret = i915_gem_object_attach_phys(obj, align); > - if (ret) { > - DRM_DEBUG_KMS("failed to attach phys object\n"); > - goto fail_locked; > - } > + else > addr = obj->phys_handle->busaddr; > - } > > finish: > - if (intel_crtc->cursor_bo) { > - if (!INTEL_INFO(dev)->cursor_needs_physical) > - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > - } > - > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > mutex_unlock(&dev->struct_mutex); > > old_width = intel_crtc->cursor_width; > @@ -8379,13 +8327,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > > intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > } > - > - return 0; > -fail_unpin: > - i915_gem_object_unpin_from_display_plane(obj); > -fail_locked: > - mutex_unlock(&dev->struct_mutex); > - return ret; > } > > static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > @@ -11544,7 +11485,6 @@ disable_unpin: > mutex_lock(&dev->struct_mutex); > i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); > - intel_unpin_fb_obj(intel_fb_obj(plane->fb)); > mutex_unlock(&dev->struct_mutex); > plane->fb = NULL; > > @@ -11568,42 +11508,6 @@ intel_check_primary_plane(struct drm_plane *plane, > false, true, &state->visible); > } > > -static int > -intel_prepare_primary_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_crtc *crtc = state->base.crtc; > - struct drm_framebuffer *fb = state->base.fb; > - struct drm_device *dev = crtc->dev; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > - int ret; > - > - intel_crtc_wait_for_pending_flips(crtc); > - > - if (intel_crtc_has_pending_flip(crtc)) { > - DRM_ERROR("pipe is still busy with an old pageflip\n"); > - return -EBUSY; > - } > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret != 0) { > - DRM_DEBUG_KMS("pin & fence failed\n"); > - return ret; > - } > - } > - > - return 0; > -} > - > static void > intel_commit_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -11614,9 +11518,7 @@ intel_commit_primary_plane(struct drm_plane *plane, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *old_fb = plane->fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_rect *src = &state->src; > > @@ -11687,62 +11589,6 @@ intel_commit_primary_plane(struct drm_plane *plane, > intel_update_fbc(dev); > mutex_unlock(&dev->struct_mutex); > } > - > - if (old_fb && old_fb != fb) { > - if (intel_crtc->active) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - mutex_unlock(&dev->struct_mutex); > - } > -} > - > -static int > -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_plane_state state; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_primary_plane(plane, &state); > - if (ret) > - return ret; > - > - ret = intel_prepare_primary_plane(plane, &state); > - if (ret) > - return ret; > - > - intel_commit_primary_plane(plane, &state); > - > - return 0; > } > > /* Common destruction function for both primary and cursor planes */ > @@ -11755,8 +11601,8 @@ void intel_plane_destroy(struct drm_plane *plane) > } > > static const struct drm_plane_funcs intel_primary_plane_funcs = { > - .update_plane = intel_primary_plane_setplane, > - .disable_plane = intel_primary_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > @@ -11816,15 +11662,22 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > return &primary->base; > } > > -static int > +static void > intel_cursor_plane_disable(struct drm_plane *plane) > { > + struct intel_plane *intel_plane = to_intel_plane(plane); > + > if (!plane->fb) > - return 0; > + return; > > BUG_ON(!plane->crtc); > > - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + > + mutex_lock(&plane->dev->struct_mutex); > + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > + INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe)); > + mutex_unlock(&plane->dev->struct_mutex); > } > > static int > @@ -11838,7 +11691,6 @@ intel_check_cursor_plane(struct drm_plane *plane, > struct drm_rect *src = &state->src; > const struct drm_rect *clip = &state->clip; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - int crtc_w, crtc_h; > unsigned stride; > int ret; > > @@ -11856,15 +11708,14 @@ intel_check_cursor_plane(struct drm_plane *plane, > return 0; > > /* Check for which cursor types we support */ > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { > - DRM_DEBUG("Cursor dimension not supported\n"); > + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { > + DRM_DEBUG("Cursor dimension %dx%d not supported\n", > + state->base.crtc_w, state->base.crtc_h); > return -EINVAL; > } > > - stride = roundup_pow_of_two(crtc_w) * 4; > - if (obj->base.size < stride * crtc_h) { > + stride = roundup_pow_of_two(state->base.crtc_w) * 4; > + if (obj->base.size < stride * state->base.crtc_h) { > DRM_DEBUG_KMS("buffer is too small\n"); > return -ENOMEM; > } > @@ -11883,7 +11734,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > return ret; > } > > -static int > +static void > intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > @@ -11893,10 +11744,9 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane *intel_plane = to_intel_plane(plane); > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > struct drm_i915_gem_object *obj = intel_fb->obj; > - int crtc_w, crtc_h; > > - crtc->cursor_x = state->orig_dst.x1; > - crtc->cursor_y = state->orig_dst.y1; > + crtc->cursor_x = state->base.crtc_x; > + crtc->cursor_y = state->base.crtc_y; > > intel_plane->crtc_x = state->orig_dst.x1; > intel_plane->crtc_y = state->orig_dst.y1; > @@ -11909,63 +11759,20 @@ intel_commit_cursor_plane(struct drm_plane *plane, > intel_plane->obj = obj; > > if (fb != crtc->cursor->fb) { > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); > + intel_crtc_cursor_set_obj(crtc, obj, > + state->base.crtc_w, > + state->base.crtc_h); > } else { > intel_crtc_update_cursor(crtc, state->visible); > > intel_frontbuffer_flip(crtc->dev, > INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); > - > - return 0; > } > } > > -static int > -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane_state state; > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_cursor_plane(plane, &state); > - if (ret) > - return ret; > - > - return intel_commit_cursor_plane(plane, &state); > -} > - > static const struct drm_plane_funcs intel_cursor_plane_funcs = { > - .update_plane = intel_cursor_plane_update, > - .disable_plane = intel_cursor_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index d3677c3..28ccaf7 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1106,7 +1106,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > uint32_t src_x, src_y, src_w, src_h; > struct drm_rect *src = &state->src; > struct drm_rect *dst = &state->dst; > - struct drm_rect *orig_src = &state->orig_src; > const struct drm_rect *clip = &state->clip; > int hscale, vscale; > int max_scale, min_scale; > @@ -1187,10 +1186,10 @@ intel_check_sprite_plane(struct drm_plane *plane, > intel_plane->rotation); > > /* sanity check to make sure the src viewport wasn't enlarged */ > - WARN_ON(src->x1 < (int) orig_src->x1 || > - src->y1 < (int) orig_src->y1 || > - src->x2 > (int) orig_src->x2 || > - src->y2 > (int) orig_src->y2); > + WARN_ON(src->x1 < (int) state->base.src_x || > + src->y1 < (int) state->base.src_y || > + src->x2 > (int) state->base.src_x + state->base.src_w || > + src->y2 > (int) state->base.src_y + state->base.src_h); > > /* > * Hardware doesn't handle subpixel coordinates. > @@ -1258,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > return 0; > } > > -static int > -intel_prepare_sprite_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_device *dev = plane->dev; > - struct drm_crtc *crtc = state->base.crtc; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane *intel_plane = to_intel_plane(plane); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *fb = state->base.fb; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > - int ret; > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - > - /* Note that this will apply the VT-d workaround for scanouts, > - * which is more restrictive than required for sprites. (The > - * primary plane requires 256KiB alignment with 64 PTE padding, > - * the sprite planes only require 128KiB alignment and 32 PTE > - * padding. > - */ > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_SPRITE(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > static void > intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -1304,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > enum pipe pipe = intel_crtc->pipe; > struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y, src_w, src_h; > @@ -1362,68 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > if (!primary_was_enabled && primary_enabled) > intel_post_enable_primary(crtc); > } > - > - /* Unpin old obj after new one is active to avoid ugliness */ > - if (old_obj && old_obj != obj) { > - > - /* > - * It's fairly common to simply update the position of > - * an existing object. In that case, we don't need to > - * wait for vblank to avoid ugliness, we only need to > - * do the pin & ref bookkeeping. > - */ > - if (intel_crtc->active) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - mutex_unlock(&dev->struct_mutex); > - } > -} > - > -static int > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_plane_state state; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_sprite_plane(plane, &state); > - if (ret) > - return ret; > - > - ret = intel_prepare_sprite_plane(plane, &state); > - if (ret) > - return ret; > - > - intel_commit_sprite_plane(plane, &state); > - return 0; > } > > int > @@ -1459,7 +1360,6 @@ intel_sprite_plane_disable(struct drm_plane *plane) > intel_wait_for_vblank(dev, intel_plane->pipe); > > mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(intel_plane->obj); > i915_gem_track_fb(intel_plane->obj, NULL, > INTEL_FRONTBUFFER_SPRITE(pipe)); > mutex_unlock(&dev->struct_mutex); > @@ -1557,21 +1457,19 @@ int intel_plane_set_property(struct drm_plane *plane, > > int intel_plane_restore(struct drm_plane *plane) > { > - struct intel_plane *intel_plane = to_intel_plane(plane); > - > if (!plane->crtc || !plane->fb) > return 0; > > return plane->funcs->update_plane(plane, plane->crtc, plane->fb, > - intel_plane->crtc_x, intel_plane->crtc_y, > - intel_plane->crtc_w, intel_plane->crtc_h, > - intel_plane->src_x, intel_plane->src_y, > - intel_plane->src_w, intel_plane->src_h); > + plane->state->crtc_x, plane->state->crtc_y, > + plane->state->crtc_w, plane->state->crtc_h, > + plane->state->src_x, plane->state->src_y, > + plane->state->src_w, plane->state->src_h); > } > > static const struct drm_plane_funcs intel_plane_funcs = { > - .update_plane = intel_update_plane, > - .disable_plane = intel_sprite_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/8] drm/i915: Switch plane handling to atomic helpers (v2) 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper 2014-11-13 21:28 ` Bob Paauwe @ 2014-11-13 22:52 ` Matt Roper 2014-11-14 9:39 ` [Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Ville Syrjälä 2 siblings, 0 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 22:52 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Now that we have hooks to enable the atomic plane helpers, we can use the plane helpers for our .update_plane() and .disable_plane() entrypoints. Even though we'd already refactored our plane handling code into check/commit, we still have to rework some of that logic here as we make the transition. The atomic plane helpers now call prepare_fb/cleanup_fb for us, so note the following: - 'commit' should no longer unpin the old object; the cleanup_fb will take care of this for us - prepare_fb only gets called when we actually have an fb, so the internal 'disable' code needs to update frontbuffer tracking - the cursor plane code never had 'prepare' code separated from its 'commit' function, so a lot of the cursor's 'commit' needs to be dropped since prepare_fb already does it for us - with all the pinning work removed from cursor commit, it can no longer fail and should return void rather than int v2: Fix commit message typo (Bob) Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Acknowledged-by: Bob Paauwe <bob.j.paauwe@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 255 +++++------------------------------ drivers/gpu/drm/i915/intel_sprite.c | 122 ++--------------- 2 files changed, 41 insertions(+), 336 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 32a2b12..8d5a3b4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8285,17 +8285,15 @@ static bool cursor_size_ok(struct drm_device *dev, return true; } -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, - struct drm_i915_gem_object *obj, - uint32_t width, uint32_t height) +static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc, + struct drm_i915_gem_object *obj, + uint32_t width, uint32_t height) { struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; unsigned old_width; uint32_t addr; - int ret; /* if we want to turn off the cursor ignore width and height */ if (!obj) { @@ -8305,64 +8303,14 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, goto finish; } - /* we only need to pin inside GTT if cursor is non-phy */ + /* plane helper's prepare_fb() already handled pinning */ mutex_lock(&dev->struct_mutex); - if (!INTEL_INFO(dev)->cursor_needs_physical) { - unsigned alignment; - - /* - * Global gtt pte registers are special registers which actually - * forward writes to a chunk of system memory. Which means that - * there is no risk that the register values disappear as soon - * as we call intel_runtime_pm_put(), so it is correct to wrap - * only the pin/unpin/fence and not more. - */ - intel_runtime_pm_get(dev_priv); - - /* Note that the w/a also requires 2 PTE of padding following - * the bo. We currently fill all unused PTE with the shadow - * page and so we should always have valid PTE following the - * cursor preventing the VT-d warning. - */ - alignment = 0; - if (need_vtd_wa(dev)) - alignment = 64*1024; - - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); - if (ret) { - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); - intel_runtime_pm_put(dev_priv); - goto fail_locked; - } - - ret = i915_gem_object_put_fence(obj); - if (ret) { - DRM_DEBUG_KMS("failed to release fence for cursor"); - intel_runtime_pm_put(dev_priv); - goto fail_unpin; - } - + if (!INTEL_INFO(dev)->cursor_needs_physical) addr = i915_gem_obj_ggtt_offset(obj); - - intel_runtime_pm_put(dev_priv); - } else { - int align = IS_I830(dev) ? 16 * 1024 : 256; - ret = i915_gem_object_attach_phys(obj, align); - if (ret) { - DRM_DEBUG_KMS("failed to attach phys object\n"); - goto fail_locked; - } + else addr = obj->phys_handle->busaddr; - } finish: - if (intel_crtc->cursor_bo) { - if (!INTEL_INFO(dev)->cursor_needs_physical) - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); - } - - i915_gem_track_fb(intel_crtc->cursor_bo, obj, - INTEL_FRONTBUFFER_CURSOR(pipe)); mutex_unlock(&dev->struct_mutex); old_width = intel_crtc->cursor_width; @@ -8379,13 +8327,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); } - - return 0; -fail_unpin: - i915_gem_object_unpin_from_display_plane(obj); -fail_locked: - mutex_unlock(&dev->struct_mutex); - return ret; } static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, @@ -11544,7 +11485,6 @@ disable_unpin: mutex_lock(&dev->struct_mutex); i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); - intel_unpin_fb_obj(intel_fb_obj(plane->fb)); mutex_unlock(&dev->struct_mutex); plane->fb = NULL; @@ -11568,42 +11508,6 @@ intel_check_primary_plane(struct drm_plane *plane, false, true, &state->visible); } -static int -intel_prepare_primary_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_crtc *crtc = state->base.crtc; - struct drm_framebuffer *fb = state->base.fb; - struct drm_device *dev = crtc->dev; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); - int ret; - - intel_crtc_wait_for_pending_flips(crtc); - - if (intel_crtc_has_pending_flip(crtc)) { - DRM_ERROR("pipe is still busy with an old pageflip\n"); - return -EBUSY; - } - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret != 0) { - DRM_DEBUG_KMS("pin & fence failed\n"); - return ret; - } - } - - return 0; -} - static void intel_commit_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -11614,9 +11518,7 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *old_fb = plane->fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_rect *src = &state->src; @@ -11687,62 +11589,6 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_update_fbc(dev); mutex_unlock(&dev->struct_mutex); } - - if (old_fb && old_fb != fb) { - if (intel_crtc->active) - intel_wait_for_vblank(dev, intel_crtc->pipe); - - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } -} - -static int -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_plane_state state; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_primary_plane(plane, &state); - if (ret) - return ret; - - ret = intel_prepare_primary_plane(plane, &state); - if (ret) - return ret; - - intel_commit_primary_plane(plane, &state); - - return 0; } /* Common destruction function for both primary and cursor planes */ @@ -11755,8 +11601,8 @@ void intel_plane_destroy(struct drm_plane *plane) } static const struct drm_plane_funcs intel_primary_plane_funcs = { - .update_plane = intel_primary_plane_setplane, - .disable_plane = intel_primary_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, @@ -11816,15 +11662,22 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, return &primary->base; } -static int +static void intel_cursor_plane_disable(struct drm_plane *plane) { + struct intel_plane *intel_plane = to_intel_plane(plane); + if (!plane->fb) - return 0; + return; BUG_ON(!plane->crtc); - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); + intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); + + mutex_lock(&plane->dev->struct_mutex); + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, + INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe)); + mutex_unlock(&plane->dev->struct_mutex); } static int @@ -11838,7 +11691,6 @@ intel_check_cursor_plane(struct drm_plane *plane, struct drm_rect *src = &state->src; const struct drm_rect *clip = &state->clip; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - int crtc_w, crtc_h; unsigned stride; int ret; @@ -11856,15 +11708,14 @@ intel_check_cursor_plane(struct drm_plane *plane, return 0; /* Check for which cursor types we support */ - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { - DRM_DEBUG("Cursor dimension not supported\n"); + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { + DRM_DEBUG("Cursor dimension %dx%d not supported\n", + state->base.crtc_w, state->base.crtc_h); return -EINVAL; } - stride = roundup_pow_of_two(crtc_w) * 4; - if (obj->base.size < stride * crtc_h) { + stride = roundup_pow_of_two(state->base.crtc_w) * 4; + if (obj->base.size < stride * state->base.crtc_h) { DRM_DEBUG_KMS("buffer is too small\n"); return -ENOMEM; } @@ -11883,7 +11734,7 @@ intel_check_cursor_plane(struct drm_plane *plane, return ret; } -static int +static void intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane_state *state) { @@ -11893,10 +11744,9 @@ intel_commit_cursor_plane(struct drm_plane *plane, struct intel_plane *intel_plane = to_intel_plane(plane); struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); struct drm_i915_gem_object *obj = intel_fb->obj; - int crtc_w, crtc_h; - crtc->cursor_x = state->orig_dst.x1; - crtc->cursor_y = state->orig_dst.y1; + crtc->cursor_x = state->base.crtc_x; + crtc->cursor_y = state->base.crtc_y; intel_plane->crtc_x = state->orig_dst.x1; intel_plane->crtc_y = state->orig_dst.y1; @@ -11909,63 +11759,20 @@ intel_commit_cursor_plane(struct drm_plane *plane, intel_plane->obj = obj; if (fb != crtc->cursor->fb) { - crtc_w = drm_rect_width(&state->orig_dst); - crtc_h = drm_rect_height(&state->orig_dst); - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); + intel_crtc_cursor_set_obj(crtc, obj, + state->base.crtc_w, + state->base.crtc_h); } else { intel_crtc_update_cursor(crtc, state->visible); intel_frontbuffer_flip(crtc->dev, INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); - - return 0; } } -static int -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_plane_state state; - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_cursor_plane(plane, &state); - if (ret) - return ret; - - return intel_commit_cursor_plane(plane, &state); -} - static const struct drm_plane_funcs intel_cursor_plane_funcs = { - .update_plane = intel_cursor_plane_update, - .disable_plane = intel_cursor_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index d3677c3..28ccaf7 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1106,7 +1106,6 @@ intel_check_sprite_plane(struct drm_plane *plane, uint32_t src_x, src_y, src_w, src_h; struct drm_rect *src = &state->src; struct drm_rect *dst = &state->dst; - struct drm_rect *orig_src = &state->orig_src; const struct drm_rect *clip = &state->clip; int hscale, vscale; int max_scale, min_scale; @@ -1187,10 +1186,10 @@ intel_check_sprite_plane(struct drm_plane *plane, intel_plane->rotation); /* sanity check to make sure the src viewport wasn't enlarged */ - WARN_ON(src->x1 < (int) orig_src->x1 || - src->y1 < (int) orig_src->y1 || - src->x2 > (int) orig_src->x2 || - src->y2 > (int) orig_src->y2); + WARN_ON(src->x1 < (int) state->base.src_x || + src->y1 < (int) state->base.src_y || + src->x2 > (int) state->base.src_x + state->base.src_w || + src->y2 > (int) state->base.src_y + state->base.src_h); /* * Hardware doesn't handle subpixel coordinates. @@ -1258,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, return 0; } -static int -intel_prepare_sprite_plane(struct drm_plane *plane, - struct intel_plane_state *state) -{ - struct drm_device *dev = plane->dev; - struct drm_crtc *crtc = state->base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_plane *intel_plane = to_intel_plane(plane); - enum pipe pipe = intel_crtc->pipe; - struct drm_framebuffer *fb = state->base.fb; - struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_plane->obj; - int ret; - - if (old_obj != obj) { - mutex_lock(&dev->struct_mutex); - - /* Note that this will apply the VT-d workaround for scanouts, - * which is more restrictive than required for sprites. (The - * primary plane requires 256KiB alignment with 64 PTE padding, - * the sprite planes only require 128KiB alignment and 32 PTE - * padding. - */ - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); - if (ret == 0) - i915_gem_track_fb(old_obj, obj, - INTEL_FRONTBUFFER_SPRITE(pipe)); - mutex_unlock(&dev->struct_mutex); - if (ret) - return ret; - } - - return 0; -} - static void intel_commit_sprite_plane(struct drm_plane *plane, struct intel_plane_state *state) @@ -1304,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, enum pipe pipe = intel_crtc->pipe; struct drm_framebuffer *fb = state->base.fb; struct drm_i915_gem_object *obj = intel_fb_obj(fb); - struct drm_i915_gem_object *old_obj = intel_plane->obj; int crtc_x, crtc_y; unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; @@ -1362,68 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, if (!primary_was_enabled && primary_enabled) intel_post_enable_primary(crtc); } - - /* Unpin old obj after new one is active to avoid ugliness */ - if (old_obj && old_obj != obj) { - - /* - * It's fairly common to simply update the position of - * an existing object. In that case, we don't need to - * wait for vblank to avoid ugliness, we only need to - * do the pin & ref bookkeeping. - */ - if (intel_crtc->active) - intel_wait_for_vblank(dev, intel_crtc->pipe); - - mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(old_obj); - mutex_unlock(&dev->struct_mutex); - } -} - -static int -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, - struct drm_framebuffer *fb, int crtc_x, int crtc_y, - unsigned int crtc_w, unsigned int crtc_h, - uint32_t src_x, uint32_t src_y, - uint32_t src_w, uint32_t src_h) -{ - struct intel_plane_state state; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int ret; - - state.base.crtc = crtc; - state.base.fb = fb; - - /* sample coordinates in 16.16 fixed point */ - state.src.x1 = src_x; - state.src.x2 = src_x + src_w; - state.src.y1 = src_y; - state.src.y2 = src_y + src_h; - - /* integer pixels */ - state.dst.x1 = crtc_x; - state.dst.x2 = crtc_x + crtc_w; - state.dst.y1 = crtc_y; - state.dst.y2 = crtc_y + crtc_h; - - state.clip.x1 = 0; - state.clip.y1 = 0; - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; - state.orig_src = state.src; - state.orig_dst = state.dst; - - ret = intel_check_sprite_plane(plane, &state); - if (ret) - return ret; - - ret = intel_prepare_sprite_plane(plane, &state); - if (ret) - return ret; - - intel_commit_sprite_plane(plane, &state); - return 0; } int @@ -1459,7 +1360,6 @@ intel_sprite_plane_disable(struct drm_plane *plane) intel_wait_for_vblank(dev, intel_plane->pipe); mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(intel_plane->obj); i915_gem_track_fb(intel_plane->obj, NULL, INTEL_FRONTBUFFER_SPRITE(pipe)); mutex_unlock(&dev->struct_mutex); @@ -1557,21 +1457,19 @@ int intel_plane_set_property(struct drm_plane *plane, int intel_plane_restore(struct drm_plane *plane) { - struct intel_plane *intel_plane = to_intel_plane(plane); - if (!plane->crtc || !plane->fb) return 0; return plane->funcs->update_plane(plane, plane->crtc, plane->fb, - intel_plane->crtc_x, intel_plane->crtc_y, - intel_plane->crtc_w, intel_plane->crtc_h, - intel_plane->src_x, intel_plane->src_y, - intel_plane->src_w, intel_plane->src_h); + plane->state->crtc_x, plane->state->crtc_y, + plane->state->crtc_w, plane->state->crtc_h, + plane->state->src_x, plane->state->src_y, + plane->state->src_w, plane->state->src_h); } static const struct drm_plane_funcs intel_plane_funcs = { - .update_plane = intel_update_plane, - .disable_plane = intel_sprite_plane_disable, + .update_plane = drm_plane_helper_update, + .disable_plane = drm_plane_helper_disable, .destroy = intel_plane_destroy, .set_property = intel_plane_set_property, .atomic_duplicate_state = intel_plane_duplicate_state, -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper 2014-11-13 21:28 ` Bob Paauwe 2014-11-13 22:52 ` [PATCH 6/8] drm/i915: Switch plane handling to atomic helpers (v2) Matt Roper @ 2014-11-14 9:39 ` Ville Syrjälä 2 siblings, 0 replies; 29+ messages in thread From: Ville Syrjälä @ 2014-11-14 9:39 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, Nov 13, 2014 at 10:43:25AM -0800, Matt Roper wrote: > Now that we have hooks to enable the atomic plane helpers, we can use > the plane helpers for our .update_plane() and .disable_plane() > entrypoints. > > Even though we'd already refactored our plane handling code into > check/commit, we still have to rework some of that logic here as we make > the transition. The atomic plane helpers now call prepare_fb/cleanup_fb > for us, so note the following: > - 'commit' should not longer unpin the old object; the cleanup_fb will > take care of this for us > - prepare_fb only gets called when we actually have an fb, so the > internal 'disable' code needs to update frontbuffer tracking > - the cursor plane code never had 'prepare' code separated from its > 'commit' function, so a lot of the cursor's 'commit' needs to be > dropped since prepare_fb already does it for us > - with all the pinning work removed from cursor commit, it can no > longer fail and should return void rather than int > > Testcase: igt/kms_plane > Testcase: igt/kms_universal_plane > Testcase: igt/kms_cursor_crc > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 255 +++++------------------------------ > drivers/gpu/drm/i915/intel_sprite.c | 122 ++--------------- > 2 files changed, 41 insertions(+), 336 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 32a2b12..8d5a3b4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8285,17 +8285,15 @@ static bool cursor_size_ok(struct drm_device *dev, > return true; > } > > -static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > - struct drm_i915_gem_object *obj, > - uint32_t width, uint32_t height) > +static void intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > + struct drm_i915_gem_object *obj, > + uint32_t width, uint32_t height) > { > struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum pipe pipe = intel_crtc->pipe; > unsigned old_width; > uint32_t addr; > - int ret; > > /* if we want to turn off the cursor ignore width and height */ > if (!obj) { > @@ -8305,64 +8303,14 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > goto finish; > } > > - /* we only need to pin inside GTT if cursor is non-phy */ > + /* plane helper's prepare_fb() already handled pinning */ > mutex_lock(&dev->struct_mutex); > - if (!INTEL_INFO(dev)->cursor_needs_physical) { > - unsigned alignment; > - > - /* > - * Global gtt pte registers are special registers which actually > - * forward writes to a chunk of system memory. Which means that > - * there is no risk that the register values disappear as soon > - * as we call intel_runtime_pm_put(), so it is correct to wrap > - * only the pin/unpin/fence and not more. > - */ > - intel_runtime_pm_get(dev_priv); > - > - /* Note that the w/a also requires 2 PTE of padding following > - * the bo. We currently fill all unused PTE with the shadow > - * page and so we should always have valid PTE following the > - * cursor preventing the VT-d warning. > - */ > - alignment = 0; > - if (need_vtd_wa(dev)) > - alignment = 64*1024; > - > - ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); > - if (ret) { > - DRM_DEBUG_KMS("failed to move cursor bo into the GTT\n"); > - intel_runtime_pm_put(dev_priv); > - goto fail_locked; > - } > - > - ret = i915_gem_object_put_fence(obj); > - if (ret) { > - DRM_DEBUG_KMS("failed to release fence for cursor"); > - intel_runtime_pm_put(dev_priv); > - goto fail_unpin; > - } > - So I'm now wondering where all this code is disappearing into? Anyway this conflicts pretty harshly with Gutavo's pending patches. I think at least the particular patch touching this had the r-b already (apart from a stale bo unre left behind, which can easily be fixed up when applying the patch). So I would like to get Gustavo's patches in first, especially as they move the code already to the right direction. We also have bugs in the current code that need to fixed. Gustavo's patches came close, but not quite there IIRC. So I think it would be nice to get that stuff done first, and then toss this atomic stuff on top. > + if (!INTEL_INFO(dev)->cursor_needs_physical) > addr = i915_gem_obj_ggtt_offset(obj); > - > - intel_runtime_pm_put(dev_priv); > - } else { > - int align = IS_I830(dev) ? 16 * 1024 : 256; > - ret = i915_gem_object_attach_phys(obj, align); > - if (ret) { > - DRM_DEBUG_KMS("failed to attach phys object\n"); > - goto fail_locked; > - } > + else > addr = obj->phys_handle->busaddr; > - } > > finish: > - if (intel_crtc->cursor_bo) { > - if (!INTEL_INFO(dev)->cursor_needs_physical) > - i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo); > - } > - > - i915_gem_track_fb(intel_crtc->cursor_bo, obj, > - INTEL_FRONTBUFFER_CURSOR(pipe)); > mutex_unlock(&dev->struct_mutex); > > old_width = intel_crtc->cursor_width; > @@ -8379,13 +8327,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, > > intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_CURSOR(pipe)); > } > - > - return 0; > -fail_unpin: > - i915_gem_object_unpin_from_display_plane(obj); > -fail_locked: > - mutex_unlock(&dev->struct_mutex); > - return ret; > } > > static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > @@ -11544,7 +11485,6 @@ disable_unpin: > mutex_lock(&dev->struct_mutex); > i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe)); > - intel_unpin_fb_obj(intel_fb_obj(plane->fb)); > mutex_unlock(&dev->struct_mutex); > plane->fb = NULL; > > @@ -11568,42 +11508,6 @@ intel_check_primary_plane(struct drm_plane *plane, > false, true, &state->visible); > } > > -static int > -intel_prepare_primary_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_crtc *crtc = state->base.crtc; > - struct drm_framebuffer *fb = state->base.fb; > - struct drm_device *dev = crtc->dev; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > - int ret; > - > - intel_crtc_wait_for_pending_flips(crtc); > - > - if (intel_crtc_has_pending_flip(crtc)) { > - DRM_ERROR("pipe is still busy with an old pageflip\n"); > - return -EBUSY; > - } > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_PRIMARY(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret != 0) { > - DRM_DEBUG_KMS("pin & fence failed\n"); > - return ret; > - } > - } > - > - return 0; > -} > - > static void > intel_commit_primary_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -11614,9 +11518,7 @@ intel_commit_primary_plane(struct drm_plane *plane, > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *old_fb = plane->fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb); > struct intel_plane *intel_plane = to_intel_plane(plane); > struct drm_rect *src = &state->src; > > @@ -11687,62 +11589,6 @@ intel_commit_primary_plane(struct drm_plane *plane, > intel_update_fbc(dev); > mutex_unlock(&dev->struct_mutex); > } > - > - if (old_fb && old_fb != fb) { > - if (intel_crtc->active) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - mutex_unlock(&dev->struct_mutex); > - } > -} > - > -static int > -intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_plane_state state; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_primary_plane(plane, &state); > - if (ret) > - return ret; > - > - ret = intel_prepare_primary_plane(plane, &state); > - if (ret) > - return ret; > - > - intel_commit_primary_plane(plane, &state); > - > - return 0; > } > > /* Common destruction function for both primary and cursor planes */ > @@ -11755,8 +11601,8 @@ void intel_plane_destroy(struct drm_plane *plane) > } > > static const struct drm_plane_funcs intel_primary_plane_funcs = { > - .update_plane = intel_primary_plane_setplane, > - .disable_plane = intel_primary_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > @@ -11816,15 +11662,22 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev, > return &primary->base; > } > > -static int > +static void > intel_cursor_plane_disable(struct drm_plane *plane) > { > + struct intel_plane *intel_plane = to_intel_plane(plane); > + > if (!plane->fb) > - return 0; > + return; > > BUG_ON(!plane->crtc); > > - return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0); > + > + mutex_lock(&plane->dev->struct_mutex); > + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL, > + INTEL_FRONTBUFFER_CURSOR(intel_plane->pipe)); > + mutex_unlock(&plane->dev->struct_mutex); > } > > static int > @@ -11838,7 +11691,6 @@ intel_check_cursor_plane(struct drm_plane *plane, > struct drm_rect *src = &state->src; > const struct drm_rect *clip = &state->clip; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - int crtc_w, crtc_h; > unsigned stride; > int ret; > > @@ -11856,15 +11708,14 @@ intel_check_cursor_plane(struct drm_plane *plane, > return 0; > > /* Check for which cursor types we support */ > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - if (!cursor_size_ok(dev, crtc_w, crtc_h)) { > - DRM_DEBUG("Cursor dimension not supported\n"); > + if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) { > + DRM_DEBUG("Cursor dimension %dx%d not supported\n", > + state->base.crtc_w, state->base.crtc_h); > return -EINVAL; > } > > - stride = roundup_pow_of_two(crtc_w) * 4; > - if (obj->base.size < stride * crtc_h) { > + stride = roundup_pow_of_two(state->base.crtc_w) * 4; > + if (obj->base.size < stride * state->base.crtc_h) { > DRM_DEBUG_KMS("buffer is too small\n"); > return -ENOMEM; > } > @@ -11883,7 +11734,7 @@ intel_check_cursor_plane(struct drm_plane *plane, > return ret; > } > > -static int > +static void > intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane_state *state) > { > @@ -11893,10 +11744,9 @@ intel_commit_cursor_plane(struct drm_plane *plane, > struct intel_plane *intel_plane = to_intel_plane(plane); > struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb); > struct drm_i915_gem_object *obj = intel_fb->obj; > - int crtc_w, crtc_h; > > - crtc->cursor_x = state->orig_dst.x1; > - crtc->cursor_y = state->orig_dst.y1; > + crtc->cursor_x = state->base.crtc_x; > + crtc->cursor_y = state->base.crtc_y; > > intel_plane->crtc_x = state->orig_dst.x1; > intel_plane->crtc_y = state->orig_dst.y1; > @@ -11909,63 +11759,20 @@ intel_commit_cursor_plane(struct drm_plane *plane, > intel_plane->obj = obj; > > if (fb != crtc->cursor->fb) { > - crtc_w = drm_rect_width(&state->orig_dst); > - crtc_h = drm_rect_height(&state->orig_dst); > - return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h); > + intel_crtc_cursor_set_obj(crtc, obj, > + state->base.crtc_w, > + state->base.crtc_h); > } else { > intel_crtc_update_cursor(crtc, state->visible); > > intel_frontbuffer_flip(crtc->dev, > INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe)); > - > - return 0; > } > } > > -static int > -intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane_state state; > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_cursor_plane(plane, &state); > - if (ret) > - return ret; > - > - return intel_commit_cursor_plane(plane, &state); > -} > - > static const struct drm_plane_funcs intel_cursor_plane_funcs = { > - .update_plane = intel_cursor_plane_update, > - .disable_plane = intel_cursor_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index d3677c3..28ccaf7 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1106,7 +1106,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > uint32_t src_x, src_y, src_w, src_h; > struct drm_rect *src = &state->src; > struct drm_rect *dst = &state->dst; > - struct drm_rect *orig_src = &state->orig_src; > const struct drm_rect *clip = &state->clip; > int hscale, vscale; > int max_scale, min_scale; > @@ -1187,10 +1186,10 @@ intel_check_sprite_plane(struct drm_plane *plane, > intel_plane->rotation); > > /* sanity check to make sure the src viewport wasn't enlarged */ > - WARN_ON(src->x1 < (int) orig_src->x1 || > - src->y1 < (int) orig_src->y1 || > - src->x2 > (int) orig_src->x2 || > - src->y2 > (int) orig_src->y2); > + WARN_ON(src->x1 < (int) state->base.src_x || > + src->y1 < (int) state->base.src_y || > + src->x2 > (int) state->base.src_x + state->base.src_w || > + src->y2 > (int) state->base.src_y + state->base.src_h); > > /* > * Hardware doesn't handle subpixel coordinates. > @@ -1258,41 +1257,6 @@ intel_check_sprite_plane(struct drm_plane *plane, > return 0; > } > > -static int > -intel_prepare_sprite_plane(struct drm_plane *plane, > - struct intel_plane_state *state) > -{ > - struct drm_device *dev = plane->dev; > - struct drm_crtc *crtc = state->base.crtc; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane *intel_plane = to_intel_plane(plane); > - enum pipe pipe = intel_crtc->pipe; > - struct drm_framebuffer *fb = state->base.fb; > - struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > - int ret; > - > - if (old_obj != obj) { > - mutex_lock(&dev->struct_mutex); > - > - /* Note that this will apply the VT-d workaround for scanouts, > - * which is more restrictive than required for sprites. (The > - * primary plane requires 256KiB alignment with 64 PTE padding, > - * the sprite planes only require 128KiB alignment and 32 PTE > - * padding. > - */ > - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); > - if (ret == 0) > - i915_gem_track_fb(old_obj, obj, > - INTEL_FRONTBUFFER_SPRITE(pipe)); > - mutex_unlock(&dev->struct_mutex); > - if (ret) > - return ret; > - } > - > - return 0; > -} > - > static void > intel_commit_sprite_plane(struct drm_plane *plane, > struct intel_plane_state *state) > @@ -1304,7 +1268,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > enum pipe pipe = intel_crtc->pipe; > struct drm_framebuffer *fb = state->base.fb; > struct drm_i915_gem_object *obj = intel_fb_obj(fb); > - struct drm_i915_gem_object *old_obj = intel_plane->obj; > int crtc_x, crtc_y; > unsigned int crtc_w, crtc_h; > uint32_t src_x, src_y, src_w, src_h; > @@ -1362,68 +1325,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > if (!primary_was_enabled && primary_enabled) > intel_post_enable_primary(crtc); > } > - > - /* Unpin old obj after new one is active to avoid ugliness */ > - if (old_obj && old_obj != obj) { > - > - /* > - * It's fairly common to simply update the position of > - * an existing object. In that case, we don't need to > - * wait for vblank to avoid ugliness, we only need to > - * do the pin & ref bookkeeping. > - */ > - if (intel_crtc->active) > - intel_wait_for_vblank(dev, intel_crtc->pipe); > - > - mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(old_obj); > - mutex_unlock(&dev->struct_mutex); > - } > -} > - > -static int > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > - struct drm_framebuffer *fb, int crtc_x, int crtc_y, > - unsigned int crtc_w, unsigned int crtc_h, > - uint32_t src_x, uint32_t src_y, > - uint32_t src_w, uint32_t src_h) > -{ > - struct intel_plane_state state; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int ret; > - > - state.base.crtc = crtc; > - state.base.fb = fb; > - > - /* sample coordinates in 16.16 fixed point */ > - state.src.x1 = src_x; > - state.src.x2 = src_x + src_w; > - state.src.y1 = src_y; > - state.src.y2 = src_y + src_h; > - > - /* integer pixels */ > - state.dst.x1 = crtc_x; > - state.dst.x2 = crtc_x + crtc_w; > - state.dst.y1 = crtc_y; > - state.dst.y2 = crtc_y + crtc_h; > - > - state.clip.x1 = 0; > - state.clip.y1 = 0; > - state.clip.x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0; > - state.clip.y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0; > - state.orig_src = state.src; > - state.orig_dst = state.dst; > - > - ret = intel_check_sprite_plane(plane, &state); > - if (ret) > - return ret; > - > - ret = intel_prepare_sprite_plane(plane, &state); > - if (ret) > - return ret; > - > - intel_commit_sprite_plane(plane, &state); > - return 0; > } > > int > @@ -1459,7 +1360,6 @@ intel_sprite_plane_disable(struct drm_plane *plane) > intel_wait_for_vblank(dev, intel_plane->pipe); > > mutex_lock(&dev->struct_mutex); > - intel_unpin_fb_obj(intel_plane->obj); > i915_gem_track_fb(intel_plane->obj, NULL, > INTEL_FRONTBUFFER_SPRITE(pipe)); > mutex_unlock(&dev->struct_mutex); > @@ -1557,21 +1457,19 @@ int intel_plane_set_property(struct drm_plane *plane, > > int intel_plane_restore(struct drm_plane *plane) > { > - struct intel_plane *intel_plane = to_intel_plane(plane); > - > if (!plane->crtc || !plane->fb) > return 0; > > return plane->funcs->update_plane(plane, plane->crtc, plane->fb, > - intel_plane->crtc_x, intel_plane->crtc_y, > - intel_plane->crtc_w, intel_plane->crtc_h, > - intel_plane->src_x, intel_plane->src_y, > - intel_plane->src_w, intel_plane->src_h); > + plane->state->crtc_x, plane->state->crtc_y, > + plane->state->crtc_w, plane->state->crtc_h, > + plane->state->src_x, plane->state->src_y, > + plane->state->src_w, plane->state->src_h); > } > > static const struct drm_plane_funcs intel_plane_funcs = { > - .update_plane = intel_update_plane, > - .disable_plane = intel_sprite_plane_disable, > + .update_plane = drm_plane_helper_update, > + .disable_plane = drm_plane_helper_disable, > .destroy = intel_plane_destroy, > .set_property = intel_plane_set_property, > .atomic_duplicate_state = intel_plane_duplicate_state, > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 7/7] drm/i915: Drop unused position fields 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper ` (5 preceding siblings ...) 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper @ 2014-11-13 18:43 ` Matt Roper 2014-11-13 21:28 ` Bob Paauwe 2014-11-14 7:28 ` shuang.he 2014-11-13 22:53 ` [PATCH 8/8] drm/i915: Integrate kerneldoc for atomic plane helpers Matt Roper 7 siblings, 2 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 18:43 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel The userspace-requested plane coordinates are now stored in plane->state.base (and the i915-adjusted values are stored in plane->state), so we no longer use the coordinate fields in intel_plane or the orig_{src,dst} fields in intel_plane_state. Drop them. Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 16 ---------------- drivers/gpu/drm/i915/intel_drv.h | 6 ------ drivers/gpu/drm/i915/intel_sprite.c | 8 -------- 3 files changed, 30 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8d5a3b4..a595a58 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11526,14 +11526,6 @@ intel_commit_primary_plane(struct drm_plane *plane, crtc->x = src->x1; crtc->y = src->y1; - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj; if (intel_crtc->active) { @@ -11748,14 +11740,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, crtc->cursor_x = state->base.crtc_x; crtc->cursor_y = state->base.crtc_y; - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj; if (fb != crtc->cursor->fb) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1882de1..fdebeea 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -248,8 +248,6 @@ struct intel_plane_state { struct drm_rect src; struct drm_rect dst; struct drm_rect clip; - struct drm_rect orig_src; - struct drm_rect orig_dst; bool visible; }; @@ -485,10 +483,6 @@ struct intel_plane { struct drm_i915_gem_object *obj; bool can_scale; int max_downscale; - int crtc_x, crtc_y; - unsigned int crtc_w, crtc_h; - uint32_t src_x, src_y; - uint32_t src_w, src_h; unsigned int rotation; /* Since we need to change the watermarks before/after diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 28ccaf7..9c00b68 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1282,14 +1282,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane); WARN_ON(!primary_enabled && !state->visible && intel_crtc->active); - intel_plane->crtc_x = state->orig_dst.x1; - intel_plane->crtc_y = state->orig_dst.y1; - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); - intel_plane->src_x = state->orig_src.x1; - intel_plane->src_y = state->orig_src.y1; - intel_plane->src_w = drm_rect_width(&state->orig_src); - intel_plane->src_h = drm_rect_height(&state->orig_src); intel_plane->obj = obj; if (intel_crtc->active) { -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] drm/i915: Drop unused position fields 2014-11-13 18:43 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper @ 2014-11-13 21:28 ` Bob Paauwe 2014-11-14 7:28 ` shuang.he 1 sibling, 0 replies; 29+ messages in thread From: Bob Paauwe @ 2014-11-13 21:28 UTC (permalink / raw) To: Matt Roper; +Cc: intel-gfx, dri-devel On Thu, 13 Nov 2014 10:43:26 -0800 Matt Roper <matthew.d.roper@intel.com> wrote: > The userspace-requested plane coordinates are now stored in > plane->state.base (and the i915-adjusted values are stored in > plane->state), so we no longer use the coordinate fields in intel_plane > or the orig_{src,dst} fields in intel_plane_state. Drop them. > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 16 ---------------- > drivers/gpu/drm/i915/intel_drv.h | 6 ------ > drivers/gpu/drm/i915/intel_sprite.c | 8 -------- > 3 files changed, 30 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8d5a3b4..a595a58 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11526,14 +11526,6 @@ intel_commit_primary_plane(struct drm_plane *plane, > crtc->x = src->x1; > crtc->y = src->y1; > > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > intel_plane->obj = obj; > > if (intel_crtc->active) { > @@ -11748,14 +11740,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, > crtc->cursor_x = state->base.crtc_x; > crtc->cursor_y = state->base.crtc_y; > > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > intel_plane->obj = obj; > > if (fb != crtc->cursor->fb) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 1882de1..fdebeea 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -248,8 +248,6 @@ struct intel_plane_state { > struct drm_rect src; > struct drm_rect dst; > struct drm_rect clip; > - struct drm_rect orig_src; > - struct drm_rect orig_dst; > bool visible; > }; > > @@ -485,10 +483,6 @@ struct intel_plane { > struct drm_i915_gem_object *obj; > bool can_scale; > int max_downscale; > - int crtc_x, crtc_y; > - unsigned int crtc_w, crtc_h; > - uint32_t src_x, src_y; > - uint32_t src_w, src_h; > unsigned int rotation; > > /* Since we need to change the watermarks before/after > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 28ccaf7..9c00b68 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1282,14 +1282,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, > primary_enabled = !drm_rect_equals(dst, clip) || colorkey_enabled(intel_plane); > WARN_ON(!primary_enabled && !state->visible && intel_crtc->active); > > - intel_plane->crtc_x = state->orig_dst.x1; > - intel_plane->crtc_y = state->orig_dst.y1; > - intel_plane->crtc_w = drm_rect_width(&state->orig_dst); > - intel_plane->crtc_h = drm_rect_height(&state->orig_dst); > - intel_plane->src_x = state->orig_src.x1; > - intel_plane->src_y = state->orig_src.y1; > - intel_plane->src_w = drm_rect_width(&state->orig_src); > - intel_plane->src_h = drm_rect_height(&state->orig_src); > intel_plane->obj = obj; > > if (intel_crtc->active) { _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] drm/i915: Drop unused position fields 2014-11-13 18:43 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper 2014-11-13 21:28 ` Bob Paauwe @ 2014-11-14 7:28 ` shuang.he 1 sibling, 0 replies; 29+ messages in thread From: shuang.he @ 2014-11-14 7:28 UTC (permalink / raw) To: shuang.he, intel-gfx, matthew.d.roper Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate BYT: pass/total=291/291->290/291 PNV: pass/total=356/356->356/356 ILK: pass/total=372/372->371/372 IVB: pass/total=544/546->524/546 SNB: pass/total=423/425->418/425 HSW: pass/total=579/579->570/579 BDW: pass/total=433/435->434/435 -------------------------------------Detailed------------------------------------- test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)... BYT: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(18, M36M31)PASS(1, M38) -> TIMEOUT(4, M36) ILK: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, FAIL(6, M26)DMESG_FAIL(1, M26)TIMEOUT(33, M37M6M26)PASS(1, M26) -> TIMEOUT(4, M6) IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(9, M4M21M34M45)PASS(16, M21M34M4M45) -> NSPT(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-onscreen, DMESG_WARN(3, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-random, DMESG_WARN(1, M21)TIMEOUT(2, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-sliding, DMESG_WARN(2, M21)TIMEOUT(1, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-offscreen, DMESG_WARN(3, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-onscreen, DMESG_WARN(3, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, DMESG_WARN(3, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(2, M21)TIMEOUT(1, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-offscreen, DMESG_WARN(3, M21)PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-onscreen, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-random, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-sliding, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-B-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-C-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-A-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-A-plane-2, PASS(1, M4) -> TIMEOUT(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-B-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-C-plane-1, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_rotation_crc_primary-rotation, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) IVB: Intel_gpu_tools, igt_kms_rotation_crc_sprite-rotation, PASS(1, M4) -> DMESG_WARN(1, M21)PASS(3, M21) SNB: Intel_gpu_tools, igt_gem_concurrent_blit_gpuX-bcs-overwrite-source-forked, FAIL(1, M22)PASS(21, M35M22) -> PASS(4, M22) SNB: Intel_gpu_tools, igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip, DMESG_WARN(3, M22)PASS(1, M22) -> DMESG_WARN(1, M22)PASS(3, M22) SNB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-B-plane-1, DMESG_WARN(3, M22)PASS(1, M22) -> DMESG_WARN(1, M22)PASS(3, M22) SNB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-A-plane-1, DMESG_WARN(3, M22)PASS(1, M22) -> DMESG_WARN(1, M22)PASS(3, M22) SNB: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-B-plane-1, DMESG_WARN(3, M22)PASS(1, M22) -> DMESG_WARN(1, M22)PASS(3, M22) SNB: Intel_gpu_tools, igt_kms_rotation_crc_sprite-rotation, DMESG_WARN(3, M22)PASS(1, M22) -> DMESG_WARN(1, M22)PASS(3, M22) SNB: Intel_gpu_tools, igt_pm_rpm_cursor, DMESG_WARN(3, M22)PASS(1, M35) -> DMESG_WARN(1, M22)PASS(3, M22) HSW: Intel_gpu_tools, igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-B-plane-1, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-C-plane-1, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-B-plane-1, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_plane_plane-position-hole-pipe-C-plane-2, TIMEOUT(1, M19)PASS(3, M39M19) -> TIMEOUT(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_kms_rotation_crc_primary-rotation, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_pm_rpm_cursor, DMESG_WARN(2, M19)TIMEOUT(1, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) HSW: Intel_gpu_tools, igt_pm_rpm_cursor-dpms, DMESG_WARN(3, M19)PASS(1, M39) -> DMESG_WARN(1, M19)PASS(3, M19) BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, DMESG_WARN(1, M30)PASS(24, M30M42M28) -> PASS(4, M28) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 8/8] drm/i915: Integrate kerneldoc for atomic plane helpers 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper ` (6 preceding siblings ...) 2014-11-13 18:43 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper @ 2014-11-13 22:53 ` Matt Roper 7 siblings, 0 replies; 29+ messages in thread From: Matt Roper @ 2014-11-13 22:53 UTC (permalink / raw) To: intel-gfx; +Cc: dri-devel Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- Documentation/DocBook/drm.tmpl | 5 +++++ drivers/gpu/drm/i915/intel_atomic_plane.c | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 9449cd6..fefee7d 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3879,6 +3879,11 @@ int num_ioctls;</synopsis> </para> </sect2> <sect2> + <title>Atomic Plane Helpers</title> +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers +!Idrivers/gpu/drm/i915/intel_atomic_plane.c + </sect2> + <sect2> <title>Output Probing</title> <para> This section covers output probing and related infrastructure like the diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 7c039bf..b0c319c 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -21,6 +21,16 @@ * DEALINGS IN THE SOFTWARE. */ +/** + * DOC: atomic plane helper support + * + * The functions here are used by the atomic plane helper functions to + * implement legacy plane updates (i.e., drm_plane->update_plane() and + * drm_plane->disable_plane()). This allows plane updates to use the + * atomic state infrastructure and perform plane updates as separate + * prepare/check/commit/cleanup steps. + */ + #include <drm/drmP.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_plane_helper.h> -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 0/7] i915 atomic plane helper conversion (v3)
@ 2014-12-09 19:53 Matt Roper
2014-12-09 19:53 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper
0 siblings, 1 reply; 29+ messages in thread
From: Matt Roper @ 2014-12-09 19:53 UTC (permalink / raw)
To: intel-gfx
This series transitions the i915 driver over to using that atomic plane helpers
to handle plane updates and disables. The previous display plane refactoring
series brought us in line with the plane helper design, so the main work here
is separating work that can sleep out of the main commit entrypoints, moving
the commit under vblank evasion (i.e., interrupts disabled), and then adding
the plane helper entrypoints, which are now just thin wrappers around the
internal functions we already have.
Previous version of this patchset:
http://lists.freedesktop.org/archives/intel-gfx/2014-November/056272.html
Matt Roper (7):
drm/plane-helper: Test for plane disable earlier
drm/i915: Refactor work that can sleep out of commit (v3)
drm/i915: Move vblank evasion to commit (v3)
drm/i915: Clarify sprite plane function names (v2)
drm/i915: Prepare for atomic plane helpers (v6)
drm/i915: Switch plane handling to atomic helpers (v5)
drm/i915: Drop unused position fields
Documentation/DocBook/drm.tmpl | 5 +
drivers/gpu/drm/drm_plane_helper.c | 16 +-
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/intel_atomic_plane.c | 145 ++++++++++++
drivers/gpu/drm/i915/intel_display.c | 352 +++++++++++++++---------------
drivers/gpu/drm/i915/intel_drv.h | 49 ++++-
drivers/gpu/drm/i915/intel_sprite.c | 165 +++++---------
7 files changed, 428 insertions(+), 305 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 7/7] drm/i915: Drop unused position fields 2014-12-09 19:53 [PATCH 0/7] i915 atomic plane helper conversion (v3) Matt Roper @ 2014-12-09 19:53 ` Matt Roper 2014-12-10 7:12 ` shuang.he 0 siblings, 1 reply; 29+ messages in thread From: Matt Roper @ 2014-12-09 19:53 UTC (permalink / raw) To: intel-gfx The userspace-requested plane coordinates are now stored in plane->state.base (and the i915-adjusted values are stored in plane->state), so we no longer use the coordinate fields in intel_plane or the orig_{src,dst} fields in intel_plane_state. Drop them. Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 33 ++++++++------------------------- drivers/gpu/drm/i915/intel_drv.h | 4 ---- drivers/gpu/drm/i915/intel_sprite.c | 19 ++++--------------- 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2f80236..85b015a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9616,7 +9616,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct drm_plane *primary = crtc->primary; - struct intel_plane *intel_plane = to_intel_plane(primary); enum pipe pipe = intel_crtc->pipe; struct intel_unpin_work *work; struct intel_engine_cs *ring; @@ -9776,14 +9775,14 @@ free_work: if (ret == -EIO) { out_hang: ret = primary->funcs->update_plane(primary, crtc, fb, - intel_plane->crtc_x, - intel_plane->crtc_y, - intel_plane->crtc_h, - intel_plane->crtc_w, - intel_plane->src_x, - intel_plane->src_y, - intel_plane->src_h, - intel_plane->src_w); + primary->state->crtc_x, + primary->state->crtc_y, + primary->state->crtc_h, + primary->state->crtc_w, + primary->state->src_x, + primary->state->src_y, + primary->state->src_h, + primary->state->src_w); if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event); @@ -11827,14 +11826,6 @@ intel_commit_primary_plane(struct drm_plane *plane, crtc->x = src->x1 >> 16; crtc->y = src->y1 >> 16; - intel_plane->crtc_x = state->base.crtc_x; - intel_plane->crtc_y = state->base.crtc_y; - intel_plane->crtc_w = state->base.crtc_w; - intel_plane->crtc_h = state->base.crtc_h; - intel_plane->src_x = state->base.src_x; - intel_plane->src_y = state->base.src_y; - intel_plane->src_w = state->base.src_w; - intel_plane->src_h = state->base.src_h; intel_plane->obj = obj; if (intel_crtc->active) { @@ -12090,14 +12081,6 @@ intel_commit_cursor_plane(struct drm_plane *plane, crtc->cursor_x = state->base.crtc_x; crtc->cursor_y = state->base.crtc_y; - intel_plane->crtc_x = state->base.crtc_x; - intel_plane->crtc_y = state->base.crtc_y; - intel_plane->crtc_w = state->base.crtc_w; - intel_plane->crtc_h = state->base.crtc_h; - intel_plane->src_x = state->base.src_x; - intel_plane->src_y = state->base.src_y; - intel_plane->src_w = state->base.src_w; - intel_plane->src_h = state->base.src_h; intel_plane->obj = obj; if (intel_crtc->cursor_bo == obj) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index d8a6d6c..8de78c9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -516,10 +516,6 @@ struct intel_plane { struct drm_i915_gem_object *obj; bool can_scale; int max_downscale; - int crtc_x, crtc_y; - unsigned int crtc_w, crtc_h; - uint32_t src_x, src_y; - uint32_t src_w, src_h; unsigned int rotation; /* Since we need to change the watermarks before/after diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 7b7acb0..b3c3bfd 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1259,15 +1259,6 @@ intel_commit_sprite_plane(struct drm_plane *plane, unsigned int crtc_w, crtc_h; uint32_t src_x, src_y, src_w, src_h; - intel_plane->crtc_x = state->base.crtc_x; - intel_plane->crtc_y = state->base.crtc_y; - intel_plane->crtc_w = state->base.crtc_w; - intel_plane->crtc_h = state->base.crtc_h; - intel_plane->src_x = state->base.src_x; - intel_plane->src_y = state->base.src_y; - intel_plane->src_w = state->base.src_w; - intel_plane->src_h = state->base.src_h; - crtc = crtc ? crtc : plane->crtc; intel_crtc = to_intel_crtc(crtc); @@ -1382,16 +1373,14 @@ int intel_plane_set_property(struct drm_plane *plane, int intel_plane_restore(struct drm_plane *plane) { - struct intel_plane *intel_plane = to_intel_plane(plane); - if (!plane->crtc || !plane->fb) return 0; return plane->funcs->update_plane(plane, plane->crtc, plane->fb, - intel_plane->crtc_x, intel_plane->crtc_y, - intel_plane->crtc_w, intel_plane->crtc_h, - intel_plane->src_x, intel_plane->src_y, - intel_plane->src_w, intel_plane->src_h); + plane->state->crtc_x, plane->state->crtc_y, + plane->state->crtc_w, plane->state->crtc_h, + plane->state->src_x, plane->state->src_y, + plane->state->src_w, plane->state->src_h); } static const struct drm_plane_funcs intel_plane_funcs = { -- 1.8.5.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 7/7] drm/i915: Drop unused position fields 2014-12-09 19:53 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper @ 2014-12-10 7:12 ` shuang.he 0 siblings, 0 replies; 29+ messages in thread From: shuang.he @ 2014-12-10 7:12 UTC (permalink / raw) To: shuang.he, intel-gfx, matthew.d.roper Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK +1-6 364/366 359/366 SNB -5 448/450 443/450 IVB -7 497/498 490/498 BYT 289/289 289/289 HSW -7 563/564 556/564 BDW 417/417 417/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied ILK igt_kms_flip_nonexisting-fb DMESG_WARN(1, M26)PASS(3, M26) DMESG_WARN(1, M26) *ILK igt_kms_pipe_crc_basic_bad-source PASS(2, M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_rcs-flip-vs-panning-interruptible PASS(3, M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible PASS(5, M26) DMESG_WARN(1, M26) *ILK igt_kms_render_direct-render PASS(4, M26) DMESG_WARN(1, M26) *ILK igt_kms_flip_bcs-flip-vs-modeset-interruptible PASS(3, M26) DMESG_WARN(1, M26) ILK igt_kms_flip_wf_vblank-ts-check DMESG_WARN(2, M26)PASS(12, M26M37) PASS(1, M26) *SNB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(3, M35) DMESG_WARN(1, M35) *SNB igt_kms_plane_plane-position-covered-pipe-B-plane-1 PASS(3, M35) DMESG_WARN(1, M35) *SNB igt_kms_plane_plane-position-hole-pipe-A-plane-1 PASS(3, M35) DMESG_WARN(1, M35) *SNB igt_kms_plane_plane-position-hole-pipe-B-plane-1 PASS(3, M35) DMESG_WARN(1, M35) *SNB igt_kms_rotation_crc_sprite-rotation PASS(3, M35) DMESG_WARN(1, M35) *IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_plane_plane-position-covered-pipe-A-plane-1 PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_plane_plane-position-covered-pipe-B-plane-1 PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_plane_plane-position-hole-pipe-A-plane-1 PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_plane_plane-position-hole-pipe-B-plane-1 PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(3, M4M21) DMESG_WARN(1, M21) *IVB igt_kms_rotation_crc_sprite-rotation PASS(3, M4M21) DMESG_WARN(1, M21) *HSW igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip PASS(2, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-covered-pipe-A-plane-1 PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-covered-pipe-B-plane-1 PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-covered-pipe-C-plane-1 PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-hole-pipe-A-plane-1 PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-hole-pipe-B-plane-1 PASS(3, M40) DMESG_WARN(1, M40) *HSW igt_kms_plane_plane-position-hole-pipe-C-plane-1 PASS(3, M40) DMESG_WARN(1, M40) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2014-12-10 7:12 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-13 18:43 [PATCH 0/7] i915 atomic plane helper conversion Matt Roper 2014-11-13 18:43 ` [PATCH 1/7] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper 2014-11-13 21:25 ` Bob Paauwe 2014-11-13 18:43 ` [PATCH 2/7] drm/i915: Allow intel_plane_disable() to operate on all plane types Matt Roper 2014-11-13 19:11 ` Bob Paauwe 2014-11-13 19:12 ` Matt Roper 2014-11-13 19:44 ` Bob Paauwe 2014-11-13 19:23 ` Ville Syrjälä 2014-11-13 22:15 ` Matt Roper 2014-11-13 18:43 ` [PATCH 3/7] drm/i915: Clarify sprite plane function names Matt Roper 2014-11-13 21:26 ` [Intel-gfx] " Bob Paauwe 2014-11-13 18:43 ` [PATCH 4/7] drm/i915: Make intel_crtc_has_pending_flip() non-static Matt Roper 2014-11-13 21:27 ` Bob Paauwe 2014-11-13 18:43 ` [PATCH 5/7] drm/i915: Prepare for atomic plane helpers Matt Roper 2014-11-13 19:46 ` Bob Paauwe 2014-11-13 21:31 ` Matt Roper 2014-11-13 22:51 ` [PATCH 5/8] drm/i915: Prepare for atomic plane helpers (v2) Matt Roper 2014-11-14 1:23 ` Matt Roper 2014-11-14 7:48 ` Daniel Vetter 2014-11-13 18:43 ` [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Matt Roper 2014-11-13 21:28 ` Bob Paauwe 2014-11-13 22:52 ` [PATCH 6/8] drm/i915: Switch plane handling to atomic helpers (v2) Matt Roper 2014-11-14 9:39 ` [Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers Ville Syrjälä 2014-11-13 18:43 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper 2014-11-13 21:28 ` Bob Paauwe 2014-11-14 7:28 ` shuang.he 2014-11-13 22:53 ` [PATCH 8/8] drm/i915: Integrate kerneldoc for atomic plane helpers Matt Roper -- strict thread matches above, loose matches on Subject: below -- 2014-12-09 19:53 [PATCH 0/7] i915 atomic plane helper conversion (v3) Matt Roper 2014-12-09 19:53 ` [PATCH 7/7] drm/i915: Drop unused position fields Matt Roper 2014-12-10 7:12 ` shuang.he
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox