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;

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,

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,

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

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,

};

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);

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;

}

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; }

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,

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

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) {