* [PATCH 01/10] drm: add helper to get crtc timings (v5)
@ 2014-12-01 23:40 Matt Roper
2014-12-01 23:40 ` [PATCH 02/10] drm/i915: remove intel_crtc_cursor_set_obj() (v5) Matt Roper
` (8 more replies)
0 siblings, 9 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Gustavo Padovan, dri-devel
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
We need to get hdisplay and vdisplay in a few places so create a
helper to make our job easier.
Note that drm_crtc_check_viewport() and intel_modeset_pipe_config() were
previously making adjustments for doublescan modes and vscan > 1 modes,
which was incorrect. Using our new helper fixes this mistake.
v2 (by Matt): Use new stereo doubling function (suggested by Ville)
v3 (by Matt):
- Add missing kerneldoc (Daniel)
- Use drm_mode_copy() (Jani)
v4 (by Matt):
- Drop stereo doubling function again; add 'stereo only' flag
to drm_mode_set_crtcinfo() instead (Ville)
v5 (by Matt):
- Note behavioral change in drm_crtc_check_viewport() and
intel_modeset_pipe_config(). (Ander)
- Describe new adjustment flags in drm_mode_set_crtcinfo()'s
kerneldoc. (Ander)
Cc: dri-devel@lists.freedesktop.org
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++++++----------
drivers/gpu/drm/drm_modes.c | 26 ++++++++++++++++----------
drivers/gpu/drm/i915/intel_display.c | 6 +++---
include/drm/drm_crtc.h | 2 ++
include/drm/drm_modes.h | 3 +++
5 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index de79283..2985e3f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2550,6 +2550,27 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
EXPORT_SYMBOL(drm_mode_set_config_internal);
/**
+ * drm_crtc_get_hv_timing - Fetches hdisplay/vdisplay for given mode
+ * @mode: mode to query
+ * @hdisplay: hdisplay value to fill in
+ * @vdisplay: vdisplay value to fill in
+ *
+ * The vdisplay value will be doubled if the specified mode is a stereo mode of
+ * the appropriate layout.
+ */
+void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
+ int *hdisplay, int *vdisplay)
+{
+ struct drm_display_mode adjusted;
+
+ drm_mode_copy(&adjusted, mode);
+ drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE_ONLY);
+ *hdisplay = adjusted.crtc_hdisplay;
+ *vdisplay = adjusted.crtc_vdisplay;
+}
+EXPORT_SYMBOL(drm_crtc_get_hv_timing);
+
+/**
* drm_crtc_check_viewport - Checks that a framebuffer is big enough for the
* CRTC viewport
* @crtc: CRTC that framebuffer will be displayed on
@@ -2566,16 +2587,7 @@ int drm_crtc_check_viewport(const struct drm_crtc *crtc,
{
int hdisplay, vdisplay;
- hdisplay = mode->hdisplay;
- vdisplay = mode->vdisplay;
-
- if (drm_mode_is_stereo(mode)) {
- struct drm_display_mode adjusted = *mode;
-
- drm_mode_set_crtcinfo(&adjusted, CRTC_STEREO_DOUBLE);
- hdisplay = adjusted.crtc_hdisplay;
- vdisplay = adjusted.crtc_vdisplay;
- }
+ drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
if (crtc->invert_dimensions)
swap(hdisplay, vdisplay);
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 6d8b941..7689c14 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -739,6 +739,8 @@ EXPORT_SYMBOL(drm_mode_vrefresh);
* - The CRTC_STEREO_DOUBLE flag can be used to compute the timings for
* buffers containing two eyes (only adjust the timings when needed, eg. for
* "frame packing" or "side by side full").
+ * - The CRTC_NO_DBLSCAN and CRTC_NO_VSCAN flags request that adjustment *not*
+ * be performed for doublescan and vscan > 1 modes respectively.
*/
void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags)
{
@@ -765,18 +767,22 @@ void drm_mode_set_crtcinfo(struct drm_display_mode *p, int adjust_flags)
}
}
- if (p->flags & DRM_MODE_FLAG_DBLSCAN) {
- p->crtc_vdisplay *= 2;
- p->crtc_vsync_start *= 2;
- p->crtc_vsync_end *= 2;
- p->crtc_vtotal *= 2;
+ if (!(adjust_flags & CRTC_NO_DBLSCAN)) {
+ if (p->flags & DRM_MODE_FLAG_DBLSCAN) {
+ p->crtc_vdisplay *= 2;
+ p->crtc_vsync_start *= 2;
+ p->crtc_vsync_end *= 2;
+ p->crtc_vtotal *= 2;
+ }
}
- if (p->vscan > 1) {
- p->crtc_vdisplay *= p->vscan;
- p->crtc_vsync_start *= p->vscan;
- p->crtc_vsync_end *= p->vscan;
- p->crtc_vtotal *= p->vscan;
+ if (!(adjust_flags & CRTC_NO_VSCAN)) {
+ if (p->vscan > 1) {
+ p->crtc_vdisplay *= p->vscan;
+ p->crtc_vsync_start *= p->vscan;
+ p->crtc_vsync_end *= p->vscan;
+ p->crtc_vtotal *= p->vscan;
+ }
}
if (adjust_flags & CRTC_STEREO_DOUBLE) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c424c36..bda58f9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10275,9 +10275,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
* computation to clearly distinguish it from the adjusted mode, which
* can be changed by the connectors in the below retry loop.
*/
- drm_mode_set_crtcinfo(&pipe_config->requested_mode, CRTC_STEREO_DOUBLE);
- pipe_config->pipe_src_w = pipe_config->requested_mode.crtc_hdisplay;
- pipe_config->pipe_src_h = pipe_config->requested_mode.crtc_vdisplay;
+ drm_crtc_get_hv_timing(&pipe_config->requested_mode,
+ &pipe_config->pipe_src_w,
+ &pipe_config->pipe_src_h);
encoder_retry:
/* Ensure the port clock defaults are reset when retrying. */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index dd2c16e..969da0f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1161,6 +1161,8 @@ extern int drm_plane_init(struct drm_device *dev,
extern void drm_plane_cleanup(struct drm_plane *plane);
extern unsigned int drm_plane_index(struct drm_plane *plane);
extern void drm_plane_force_disable(struct drm_plane *plane);
+extern void drm_crtc_get_hv_timing(const struct drm_display_mode *mode,
+ int *hdisplay, int *vdisplay);
extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
int x, int y,
const struct drm_display_mode *mode,
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..8f17811 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -90,6 +90,9 @@ enum drm_mode_status {
#define CRTC_INTERLACE_HALVE_V (1 << 0) /* halve V values for interlacing */
#define CRTC_STEREO_DOUBLE (1 << 1) /* adjust timings for stereo modes */
+#define CRTC_NO_DBLSCAN (1 << 2) /* don't adjust doublescan */
+#define CRTC_NO_VSCAN (1 << 3) /* don't adjust doublescan */
+#define CRTC_STEREO_DOUBLE_ONLY (CRTC_NO_DBLSCAN | CRTC_NO_VSCAN)
#define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 02/10] drm/i915: remove intel_crtc_cursor_set_obj() (v5)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 03/10] drm/i915: remove intel_pipe_set_base() (v4) Matt Roper
` (7 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Gustavo Padovan
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Merge it into the plane update_plane() callback and make other
users use the update_plane() functions instead.
The fb != crtc->cursor->fb was already inside intel_crtc_cursor_set_obj()
so we fold intel_crtc_cursor_set_obj() inside intel_commit_cursor_plane()
and merge both paths into one.
v5 (by Matt):
- Rebase onto latest di-nightly codebase
- Drop extra unreference call when we fail to pin (Ville)
Reviewed-by(v4): Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 215 ++++++++++++++++-------------------
1 file changed, 99 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bda58f9..9f9ed54 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8417,109 +8417,6 @@ 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)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- 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) {
- DRM_DEBUG_KMS("cursor off\n");
- addr = 0;
- mutex_lock(&dev->struct_mutex);
- goto finish;
- }
-
- /* we only need to pin inside GTT if cursor is non-phy */
- 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;
- }
-
- 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;
- }
- 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;
-
- intel_crtc->cursor_addr = addr;
- intel_crtc->cursor_bo = obj;
- intel_crtc->cursor_width = width;
- intel_crtc->cursor_height = height;
-
- if (intel_crtc->active) {
- if (old_width != width)
- intel_update_watermarks(crtc);
- intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
-
- 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,
u16 *blue, uint32_t start, uint32_t size)
{
@@ -12082,7 +11979,8 @@ intel_cursor_plane_disable(struct drm_plane *plane)
BUG_ON(!plane->crtc);
- return intel_crtc_cursor_set_obj(plane->crtc, NULL, 0, 0);
+ return plane->funcs->update_plane(plane, plane->crtc, NULL,
+ 0, 0, 0, 0, 0, 0, 0, 0);
}
static int
@@ -12146,12 +12044,15 @@ 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_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
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);
- struct drm_i915_gem_object *obj = intel_fb->obj;
- int crtc_w, crtc_h;
+ struct drm_i915_gem_object *obj = intel_fb_obj(state->fb);
+ enum pipe pipe = intel_crtc->pipe;
+ unsigned old_width;
+ uint32_t addr;
+ int ret;
crtc->cursor_x = state->orig_dst.x1;
crtc->cursor_y = state->orig_dst.y1;
@@ -12166,18 +12067,100 @@ intel_commit_cursor_plane(struct drm_plane *plane,
intel_plane->src_h = drm_rect_height(&state->orig_src);
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);
+ if (intel_crtc->cursor_bo == obj)
+ goto update;
+
+ /* if we want to turn off the cursor ignore width and height */
+ if (!obj) {
+ DRM_DEBUG_KMS("cursor off\n");
+ addr = 0;
+ mutex_lock(&dev->struct_mutex);
+ goto finish;
+ }
+
+ /* we only need to pin inside GTT if cursor is non-phy */
+ 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;
+ }
+
+ addr = i915_gem_obj_ggtt_offset(obj);
+
+ intel_runtime_pm_put(dev_priv);
+
} else {
- intel_crtc_update_cursor(crtc, state->visible);
+ 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;
+ }
+ addr = obj->phys_handle->busaddr;
+ }
- intel_frontbuffer_flip(crtc->dev,
- INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe));
+finish:
+ if (intel_crtc->cursor_bo) {
+ if (!INTEL_INFO(dev)->cursor_needs_physical)
+ i915_gem_object_unpin_from_display_plane(intel_crtc->cursor_bo);
+ }
- return 0;
+ i915_gem_track_fb(intel_crtc->cursor_bo, obj,
+ INTEL_FRONTBUFFER_CURSOR(pipe));
+ mutex_unlock(&dev->struct_mutex);
+
+ intel_crtc->cursor_addr = addr;
+ intel_crtc->cursor_bo = obj;
+update:
+ old_width = intel_crtc->cursor_width;
+
+ intel_crtc->cursor_width = drm_rect_width(&state->orig_dst);
+ intel_crtc->cursor_height = drm_rect_height(&state->orig_dst);
+
+ if (intel_crtc->active) {
+ if (old_width != intel_crtc->cursor_width)
+ intel_update_watermarks(crtc);
+ intel_crtc_update_cursor(crtc, state->visible);
+
+ 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 int
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 03/10] drm/i915: remove intel_pipe_set_base() (v4)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
2014-12-01 23:40 ` [PATCH 02/10] drm/i915: remove intel_crtc_cursor_set_obj() (v5) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 04/10] drm/i915: Introduce intel_prepare_cursor_plane() (v2) Matt Roper
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx; +Cc: Gustavo Padovan
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
After some refactor intel_primary_plane_setplane() does the same
as intel_pipe_set_base() so we can get rid of it and replace the calls
with intel_primary_plane_setplane().
v2: take Ville's comments:
- get the right arguments for update_plane()
- use drm_crtc_get_hv_timing()
v3 (by Matt):
- Rebase to latest di-nightly codebase
- Use primary->funcs->update_plane() in __intel_set_mode()
- Use primary->funcs->disable_plane() in intel_crtc_disable()
v4 (by Matt):
- Drop redundant calls to intel_crtc_wait_for_pending_flips() before
calling update_plane() (Ville)
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Acked-and-mourned-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 129 ++++++++---------------------------
1 file changed, 28 insertions(+), 101 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9f9ed54..afff718 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2954,71 +2954,6 @@ static void intel_update_pipe_size(struct intel_crtc *crtc)
crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay;
}
-static int
-intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
- struct drm_framebuffer *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);
- enum pipe pipe = intel_crtc->pipe;
- struct drm_framebuffer *old_fb = crtc->primary->fb;
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- int ret;
-
- if (intel_crtc_has_pending_flip(crtc)) {
- DRM_ERROR("pipe is still busy with an old pageflip\n");
- return -EBUSY;
- }
-
- /* no fb bound */
- if (!fb) {
- DRM_ERROR("No FB bound\n");
- return 0;
- }
-
- if (intel_crtc->plane > INTEL_INFO(dev)->num_pipes) {
- DRM_ERROR("no plane for crtc: plane %c, num_pipes %d\n",
- plane_name(intel_crtc->plane),
- INTEL_INFO(dev)->num_pipes);
- return -EINVAL;
- }
-
- mutex_lock(&dev->struct_mutex);
- ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL);
- if (ret == 0)
- i915_gem_track_fb(old_obj, intel_fb_obj(fb),
- INTEL_FRONTBUFFER_PRIMARY(pipe));
- mutex_unlock(&dev->struct_mutex);
- if (ret != 0) {
- DRM_ERROR("pin & fence failed\n");
- return ret;
- }
-
- dev_priv->display.update_primary_plane(crtc, fb, x, y);
-
- if (intel_crtc->active)
- intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
-
- crtc->primary->fb = fb;
- crtc->x = x;
- crtc->y = y;
-
- if (old_fb) {
- if (intel_crtc->active && old_fb != fb)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(old_obj);
- mutex_unlock(&dev->struct_mutex);
- }
-
- mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
- mutex_unlock(&dev->struct_mutex);
-
- return 0;
-}
-
static void intel_fdi_normal_train(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -5312,8 +5247,6 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_device *dev = crtc->dev;
struct drm_connector *connector;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_i915_gem_object *old_obj = intel_fb_obj(crtc->primary->fb);
- enum pipe pipe = to_intel_crtc(crtc)->pipe;
/* crtc should still be enabled when we disable it. */
WARN_ON(!crtc->enabled);
@@ -5321,14 +5254,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
dev_priv->display.crtc_disable(crtc);
dev_priv->display.off(crtc);
- if (crtc->primary->fb) {
- mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(old_obj);
- i915_gem_track_fb(old_obj, NULL,
- INTEL_FRONTBUFFER_PRIMARY(pipe));
- mutex_unlock(&dev->struct_mutex);
- crtc->primary->fb = NULL;
- }
+ crtc->primary->funcs->disable_plane(crtc->primary);
/* Update computed state. */
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
@@ -9691,6 +9617,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *old_fb = crtc->primary->fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_plane *primary = crtc->primary;
+ struct intel_plane *intel_plane = to_intel_plane(primary);
enum pipe pipe = intel_crtc->pipe;
struct intel_unpin_work *work;
struct intel_engine_cs *ring;
@@ -9849,8 +9777,15 @@ free_work:
if (ret == -EIO) {
out_hang:
- intel_crtc_wait_for_pending_flips(crtc);
- ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
+ ret = primary->funcs->update_plane(primary, crtc, fb,
+ intel_plane->crtc_x,
+ intel_plane->crtc_y,
+ intel_plane->crtc_h,
+ intel_plane->crtc_w,
+ intel_plane->src_x,
+ intel_plane->src_y,
+ intel_plane->src_h,
+ intel_plane->src_w);
if (ret == 0 && event) {
spin_lock_irq(&dev->event_lock);
drm_send_vblank_event(dev, pipe, event);
@@ -11032,26 +10967,15 @@ static int __intel_set_mode(struct drm_crtc *crtc,
* on the DPLL.
*/
for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
- struct drm_framebuffer *old_fb = crtc->primary->fb;
- struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_plane *primary = intel_crtc->base.primary;
+ int vdisplay, hdisplay;
- mutex_lock(&dev->struct_mutex);
- ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL);
- if (ret != 0) {
- DRM_ERROR("pin & fence failed\n");
- mutex_unlock(&dev->struct_mutex);
- goto done;
- }
- if (old_fb)
- intel_unpin_fb_obj(old_obj);
- i915_gem_track_fb(old_obj, obj,
- INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
- mutex_unlock(&dev->struct_mutex);
-
- crtc->primary->fb = fb;
- crtc->x = x;
- crtc->y = y;
+ drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
+ ret = primary->funcs->update_plane(primary, &intel_crtc->base,
+ fb, 0, 0,
+ hdisplay, vdisplay,
+ x << 16, y << 16,
+ hdisplay << 16, vdisplay << 16);
}
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
@@ -11517,11 +11441,14 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-
- intel_crtc_wait_for_pending_flips(set->crtc);
-
- ret = intel_pipe_set_base(set->crtc,
- set->x, set->y, set->fb);
+ struct drm_plane *primary = set->crtc->primary;
+ int vdisplay, hdisplay;
+
+ drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
+ ret = primary->funcs->update_plane(primary, set->crtc, set->fb,
+ 0, 0, hdisplay, vdisplay,
+ set->x << 16, set->y << 16,
+ hdisplay << 16, vdisplay << 16);
/*
* We need to make sure the primary plane is re-enabled if it
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 04/10] drm/i915: Introduce intel_prepare_cursor_plane() (v2)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
2014-12-01 23:40 ` [PATCH 02/10] drm/i915: remove intel_crtc_cursor_set_obj() (v5) Matt Roper
2014-12-01 23:40 ` [PATCH 03/10] drm/i915: remove intel_pipe_set_base() (v4) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 05/10] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper
` (5 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
Primary and sprite planes have already been refactored to include a
'prepare' step which handles all the commit-time operations that could
fail (i.e., pinning buffers and such). Refactor the cursor commit in a
similar manner.
For simplicity and consistency with other plane types, we also switch to
using intel_pin_and_fence_fb_obj() to perform our pinning for
non-physical cursors. This will allow us to more easily migrate the
code into the atomic 'begin' handler in a plane-agnostic manner in a
future patchset.
v2:
- Update GEM fb tracking for physical cursors too. (Ander)
- Use intel_unpin_fb_obj() rather than
i915_gem_object_unpin_from_display_plane() and do so while holding
struct_mutex. (Ander)
- Update plane->fb in commit_cursor_plane. This isn't really necessary
since the DRM core does this for us in __setplane_internal(), but
doing it in our driver once we know we're going to succeed helps
avoid confusion. (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 124 +++++++++++++++--------------------
1 file changed, 52 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index afff718..28fe7a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11967,20 +11967,55 @@ intel_check_cursor_plane(struct drm_plane *plane,
}
static int
+intel_prepare_cursor_plane(struct drm_plane *plane,
+ struct intel_plane_state *state)
+{
+ struct drm_device *dev = plane->dev;
+ struct drm_framebuffer *fb = state->fb;
+ struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
+ enum pipe pipe = intel_crtc->pipe;
+ int ret = 0;
+
+ if (old_obj != obj) {
+ /* we only need to pin inside GTT if cursor is non-phy */
+ mutex_lock(&dev->struct_mutex);
+ if (!INTEL_INFO(dev)->cursor_needs_physical) {
+ if (obj)
+ ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
+ } else {
+ int align = IS_I830(dev) ? 16 * 1024 : 256;
+ if (obj)
+ ret = i915_gem_object_attach_phys(obj, align);
+ if (ret)
+ DRM_DEBUG_KMS("failed to attach phys object\n");
+ }
+
+ if (ret == 0)
+ i915_gem_track_fb(intel_crtc->cursor_bo, obj,
+ INTEL_FRONTBUFFER_CURSOR(pipe));
+
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ return ret;
+}
+
+static void
intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_crtc *crtc = state->crtc;
struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(state->fb);
enum pipe pipe = intel_crtc->pipe;
unsigned old_width;
uint32_t addr;
- int ret;
+ plane->fb = state->base.fb;
crtc->cursor_x = state->orig_dst.x1;
crtc->cursor_y = state->orig_dst.y1;
@@ -11997,75 +12032,21 @@ intel_commit_cursor_plane(struct drm_plane *plane,
if (intel_crtc->cursor_bo == obj)
goto update;
- /* if we want to turn off the cursor ignore width and height */
- if (!obj) {
- DRM_DEBUG_KMS("cursor off\n");
+ if (!obj)
addr = 0;
- mutex_lock(&dev->struct_mutex);
- goto finish;
- }
-
- /* we only need to pin inside GTT if cursor is non-phy */
- 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;
- }
-
+ else 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);
+ if (!INTEL_INFO(dev)->cursor_needs_physical) {
+ mutex_lock(&dev->struct_mutex);
+ intel_unpin_fb_obj(intel_crtc->cursor_bo);
+ mutex_unlock(&dev->struct_mutex);
+ }
}
- i915_gem_track_fb(intel_crtc->cursor_bo, obj,
- INTEL_FRONTBUFFER_CURSOR(pipe));
- mutex_unlock(&dev->struct_mutex);
-
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
@@ -12081,13 +12062,6 @@ update:
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 int
@@ -12128,7 +12102,13 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
if (ret)
return ret;
- return intel_commit_cursor_plane(plane, &state);
+ ret = intel_prepare_cursor_plane(plane, &state);
+ if (ret)
+ return ret;
+
+ intel_commit_cursor_plane(plane, &state);
+
+ return 0;
}
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 05/10] drm/i915: Make intel_plane_state subclass drm_plane_state
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (2 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 04/10] drm/i915: Introduce intel_prepare_cursor_plane() (v2) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 06/10] drm/i915: Consolidate plane 'prepare' functions (v2) Matt Roper
` (4 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 34 +++++++++++++++++-----------------
drivers/gpu/drm/i915/intel_drv.h | 3 +--
drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++--------
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 28fe7a8..bbc0fcd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11646,8 +11646,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;
@@ -11663,8 +11663,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;
@@ -11699,9 +11699,9 @@ 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_device *dev = crtc->dev;
+ struct drm_crtc *crtc = state->base.crtc;
+ struct drm_framebuffer *fb = state->base.fb;
+ struct drm_device *dev = plane->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;
@@ -11800,8 +11800,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;
@@ -11914,9 +11914,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;
@@ -11971,8 +11971,8 @@ intel_prepare_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_device *dev = plane->dev;
- struct drm_framebuffer *fb = state->fb;
- struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
+ struct drm_framebuffer *fb = state->base.fb;
+ struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
enum pipe pipe = intel_crtc->pipe;
@@ -12006,11 +12006,11 @@ static void
intel_commit_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 intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
- struct drm_i915_gem_object *obj = intel_fb_obj(state->fb);
+ struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
enum pipe pipe = intel_crtc->pipe;
unsigned old_width;
uint32_t addr;
@@ -12075,8 +12075,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 abb2cf4..b4d181f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -244,8 +244,7 @@ typedef struct dpll {
} intel_clock_t;
struct intel_plane_state {
- struct drm_crtc *crtc;
- struct drm_framebuffer *fb;
+ struct drm_plane_state base;
struct drm_rect src;
struct drm_rect dst;
struct drm_rect clip;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7d9c340..fc96d13 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1096,9 +1096,9 @@ static int
intel_check_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
- struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
+ struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
- struct drm_framebuffer *fb = state->fb;
+ struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
@@ -1262,11 +1262,11 @@ intel_prepare_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_device *dev = plane->dev;
- struct drm_crtc *crtc = state->crtc;
+ struct drm_crtc *crtc = state->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
enum pipe pipe = intel_crtc->pipe;
- struct drm_framebuffer *fb = state->fb;
+ struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct drm_i915_gem_object *old_obj = intel_plane->obj;
int ret;
@@ -1297,11 +1297,11 @@ intel_commit_sprite_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
struct drm_device *dev = plane->dev;
- struct drm_crtc *crtc = state->crtc;
+ struct drm_crtc *crtc = state->base.crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
enum pipe pipe = intel_crtc->pipe;
- struct drm_framebuffer *fb = state->fb;
+ struct drm_framebuffer *fb = state->base.fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct drm_i915_gem_object *old_obj = intel_plane->obj;
int crtc_x, crtc_y;
@@ -1391,8 +1391,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
- state.crtc = crtc;
- state.fb = fb;
+ state.base.crtc = crtc;
+ state.base.fb = fb;
/* sample coordinates in 16.16 fixed point */
state.src.x1 = src_x;
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 06/10] drm/i915: Consolidate plane 'prepare' functions (v2)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (3 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 05/10] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2) Matt Roper
` (3 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
The 'prepare' step for all types of planes are pretty similar;
consolidate the three 'prepare' functions into a single function. This
paves the way for future integration with the atomic plane handlers.
Note that we pull the 'wait for pending flips' functionality out of the
primary plane's prepare step and place it directly in the 'setplane'
code. When we move to the atomic plane handlers, this code will be in
the 'atomic begin' step.
v2: Update GEM fb tracking for physical cursors also (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 168 +++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_drv.h | 2 +
drivers/gpu/drm/i915/intel_sprite.c | 44 ++-------
3 files changed, 98 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bbc0fcd..f48faaa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11642,6 +11642,65 @@ intel_primary_plane_disable(struct drm_plane *plane)
return 0;
}
+/**
+ * intel_prepare_plane_fb - Prepare fb for usage on plane
+ * @plane: drm plane to prepare for
+ * @fb: framebuffer to prepare for presentation
+ *
+ * Prepares a framebuffer for usage on a display plane. Generally this
+ * involves pinning the underlying object and updating the frontbuffer tracking
+ * bits. Some older platforms need special physical address handling for
+ * cursor planes.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int
+intel_prepare_plane_fb(struct drm_plane *plane,
+ struct drm_framebuffer *fb)
+{
+ struct drm_device *dev = plane->dev;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ enum pipe pipe = intel_plane->pipe;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
+ unsigned frontbuffer_bits = 0;
+ int ret = 0;
+
+ if (WARN_ON(fb == plane->fb || !obj))
+ return 0;
+
+ switch (plane->type) {
+ case DRM_PLANE_TYPE_PRIMARY:
+ frontbuffer_bits = INTEL_FRONTBUFFER_PRIMARY(pipe);
+ break;
+ case DRM_PLANE_TYPE_CURSOR:
+ frontbuffer_bits = INTEL_FRONTBUFFER_CURSOR(pipe);
+ break;
+ case DRM_PLANE_TYPE_OVERLAY:
+ frontbuffer_bits = INTEL_FRONTBUFFER_SPRITE(pipe);
+ break;
+ }
+
+ mutex_lock(&dev->struct_mutex);
+
+ 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");
+ } else {
+ ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
+ }
+
+ if (ret == 0)
+ i915_gem_track_fb(old_obj, obj, frontbuffer_bits);
+
+ mutex_unlock(&dev->struct_mutex);
+
+ return ret;
+}
+
static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
@@ -11659,42 +11718,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)
@@ -11796,6 +11819,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -11827,9 +11851,18 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
if (ret)
return ret;
- ret = intel_prepare_primary_plane(plane, &state);
- if (ret)
- return 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 (fb != old_fb && fb) {
+ ret = intel_prepare_plane_fb(plane, fb);
+ if (ret)
+ return ret;
+ }
intel_commit_primary_plane(plane, &state);
@@ -11966,42 +11999,6 @@ intel_check_cursor_plane(struct drm_plane *plane,
return ret;
}
-static int
-intel_prepare_cursor_plane(struct drm_plane *plane,
- struct intel_plane_state *state)
-{
- struct drm_device *dev = plane->dev;
- struct drm_framebuffer *fb = state->base.fb;
- struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
- enum pipe pipe = intel_crtc->pipe;
- int ret = 0;
-
- if (old_obj != obj) {
- /* we only need to pin inside GTT if cursor is non-phy */
- mutex_lock(&dev->struct_mutex);
- if (!INTEL_INFO(dev)->cursor_needs_physical) {
- if (obj)
- ret = intel_pin_and_fence_fb_obj(plane, fb, NULL);
- } else {
- int align = IS_I830(dev) ? 16 * 1024 : 256;
- if (obj)
- ret = i915_gem_object_attach_phys(obj, align);
- if (ret)
- DRM_DEBUG_KMS("failed to attach phys object\n");
- }
-
- if (ret == 0)
- i915_gem_track_fb(intel_crtc->cursor_bo, obj,
- INTEL_FRONTBUFFER_CURSOR(pipe));
-
- mutex_unlock(&dev->struct_mutex);
- }
-
- return ret;
-}
-
static void
intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
@@ -12011,6 +12008,7 @@ intel_commit_cursor_plane(struct drm_plane *plane,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+ struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
enum pipe pipe = intel_crtc->pipe;
unsigned old_width;
uint32_t addr;
@@ -12032,6 +12030,17 @@ intel_commit_cursor_plane(struct drm_plane *plane,
if (intel_crtc->cursor_bo == obj)
goto update;
+ /*
+ * 'prepare' is only called when fb != NULL; we still need to update
+ * frontbuffer tracking for the 'disable' case here.
+ */
+ if (!obj) {
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(old_obj, NULL,
+ INTEL_FRONTBUFFER_CURSOR(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
if (!obj)
addr = 0;
else if (!INTEL_INFO(dev)->cursor_needs_physical)
@@ -12071,6 +12080,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_framebuffer *old_fb = plane->fb;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane_state state;
int ret;
@@ -12102,9 +12112,11 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
if (ret)
return ret;
- ret = intel_prepare_cursor_plane(plane, &state);
- if (ret)
- return ret;
+ if (fb != old_fb && fb) {
+ ret = intel_prepare_plane_fb(plane, fb);
+ if (ret)
+ return ret;
+ }
intel_commit_cursor_plane(plane, &state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b4d181f..a36f696 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -922,6 +922,8 @@ 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);
+int intel_prepare_plane_fb(struct drm_plane *plane,
+ struct drm_framebuffer *fb);
/* shared dpll functions */
struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index fc96d13..5d8c2e0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1257,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)
@@ -1387,6 +1352,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -1417,9 +1383,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (ret)
return ret;
- ret = intel_prepare_sprite_plane(plane, &state);
- if (ret)
- return ret;
+ if (fb != old_fb && fb) {
+ ret = intel_prepare_plane_fb(plane, fb);
+ if (ret)
+ return ret;
+ }
intel_commit_sprite_plane(plane, &state);
return 0;
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (4 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 06/10] drm/i915: Consolidate plane 'prepare' functions (v2) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-02 14:10 ` Ander Conselvan de Oliveira
2014-12-01 23:40 ` [PATCH 08/10] drm/i915: Consolidate top-level .update_plane() handlers Matt Roper
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
All plane update functions need to unpin the old framebuffer when
flipping to a new one. Pull this logic into a separate function to ease
the integration with atomic plane helpers.
v2: Don't wait for vblank if we don't have an old fb to cleanup (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 27 ++++++-----------
3 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f48faaa..98e4fbf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11701,6 +11701,31 @@ intel_prepare_plane_fb(struct drm_plane *plane,
return ret;
}
+/**
+ * intel_cleanup_plane_fb - Cleans up an fb after plane use
+ * @plane: drm plane to clean up for
+ * @fb: old framebuffer that was on plane
+ *
+ * Cleans up a framebuffer that has just been removed from a plane.
+ */
+void
+intel_cleanup_plane_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);
+
+ if (WARN_ON(!obj))
+ return;
+
+ if (plane->type != DRM_PLANE_TYPE_CURSOR ||
+ !INTEL_INFO(dev)->cursor_needs_physical) {
+ mutex_lock(&dev->struct_mutex);
+ intel_unpin_fb_obj(obj);
+ mutex_unlock(&dev->struct_mutex);
+ }
+}
+
static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
@@ -11728,9 +11753,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;
@@ -11801,15 +11824,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
@@ -11819,6 +11833,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -11866,6 +11881,12 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
intel_commit_primary_plane(plane, &state);
+ if (fb != old_fb && old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
@@ -12048,14 +12069,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
else
addr = obj->phys_handle->busaddr;
- if (intel_crtc->cursor_bo) {
- if (!INTEL_INFO(dev)->cursor_needs_physical) {
- mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(intel_crtc->cursor_bo);
- mutex_unlock(&dev->struct_mutex);
- }
- }
-
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
@@ -12080,6 +12093,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane_state state;
@@ -12120,6 +12134,13 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
intel_commit_cursor_plane(plane, &state);
+ if (fb != old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ if (old_fb)
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a36f696..38cb553 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -924,6 +924,8 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
void intel_check_page_flip(struct drm_device *dev, int pipe);
int intel_prepare_plane_fb(struct drm_plane *plane,
struct drm_framebuffer *fb);
+void intel_cleanup_plane_fb(struct drm_plane *plane,
+ struct drm_framebuffer *fb);
/* shared dpll functions */
struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5d8c2e0..152a32d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1268,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;
@@ -1326,23 +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
@@ -1352,6 +1334,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -1390,6 +1373,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
}
intel_commit_sprite_plane(plane, &state);
+
+ if (fb != old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ if (old_fb)
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 08/10] drm/i915: Consolidate top-level .update_plane() handlers
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (5 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2) Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 09/10] drm/i915: Ensure state->crtc is non-NULL for plane updates Matt Roper
2014-12-01 23:40 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2) Matt Roper
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
Our .update_plane() handlers do the same check/prepare/commit/cleanup
steps regardless of plane type. Consolidate them all into a single
function that calls check/commit through a vtable.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 113 ++++++++++-------------------------
drivers/gpu/drm/i915/intel_drv.h | 9 +++
drivers/gpu/drm/i915/intel_sprite.c | 59 +-----------------
3 files changed, 44 insertions(+), 137 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 98e4fbf..f628a28 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11735,12 +11735,23 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = &state->dst;
struct drm_rect *src = &state->src;
const struct drm_rect *clip = &state->clip;
+ int ret;
+
+ ret = drm_plane_helper_check_update(plane, crtc, fb,
+ src, dest, clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true, &state->visible);
+ if (ret)
+ return ret;
- return drm_plane_helper_check_update(plane, crtc, fb,
- src, dest, clip,
- DRM_PLANE_HELPER_NO_SCALING,
- DRM_PLANE_HELPER_NO_SCALING,
- false, true, &state->visible);
+ 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;
+ }
+
+ return 0;
}
static void
@@ -11826,16 +11837,17 @@ intel_commit_primary_plane(struct drm_plane *plane,
}
}
-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)
+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 drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
+ struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
@@ -11862,24 +11874,17 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
state.orig_src = state.src;
state.orig_dst = state.dst;
- ret = intel_check_primary_plane(plane, &state);
+ ret = intel_plane->check_plane(plane, &state);
if (ret)
return 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 (fb != old_fb && fb) {
ret = intel_prepare_plane_fb(plane, fb);
if (ret)
return ret;
}
- intel_commit_primary_plane(plane, &state);
+ intel_plane->commit_plane(plane, &state);
if (fb != old_fb && old_fb) {
if (intel_crtc->active)
@@ -11887,6 +11892,8 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
intel_cleanup_plane_fb(plane, old_fb);
}
+ plane->fb = fb;
+
return 0;
}
@@ -11899,7 +11906,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
}
static const struct drm_plane_funcs intel_primary_plane_funcs = {
- .update_plane = intel_primary_plane_setplane,
+ .update_plane = intel_update_plane,
.disable_plane = intel_primary_plane_disable,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property
@@ -11921,6 +11928,8 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
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;
@@ -12086,66 +12095,8 @@ update:
}
}
-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 drm_device *dev = plane->dev;
- struct drm_framebuffer *old_fb = plane->fb;
- 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;
-
- if (fb != old_fb && fb) {
- ret = intel_prepare_plane_fb(plane, fb);
- if (ret)
- return ret;
- }
-
- intel_commit_cursor_plane(plane, &state);
-
- if (fb != old_fb) {
- if (intel_crtc->active)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- if (old_fb)
- intel_cleanup_plane_fb(plane, old_fb);
- }
-
- return 0;
-}
-
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
- .update_plane = intel_cursor_plane_update,
+ .update_plane = intel_update_plane,
.disable_plane = intel_cursor_plane_disable,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
@@ -12165,6 +12116,8 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
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,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 38cb553..f7b6619 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -508,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,
@@ -1011,6 +1015,11 @@ void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
void intel_dp_unpack_aux(uint32_t src, uint8_t *dst, int dst_bytes);
+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);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 152a32d..bfd5270 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1328,63 +1328,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
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 drm_device *dev = plane->dev;
- struct drm_framebuffer *old_fb = plane->fb;
- 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;
-
- if (fb != old_fb && fb) {
- ret = intel_prepare_plane_fb(plane, fb);
- if (ret)
- return ret;
- }
-
- intel_commit_sprite_plane(plane, &state);
-
- if (fb != old_fb) {
- if (intel_crtc->active)
- intel_wait_for_vblank(dev, intel_crtc->pipe);
- if (old_fb)
- intel_cleanup_plane_fb(plane, old_fb);
- }
-
- return 0;
-}
-
-static int
intel_disable_plane(struct drm_plane *plane)
{
struct drm_device *dev = plane->dev;
@@ -1679,6 +1622,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,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 09/10] drm/i915: Ensure state->crtc is non-NULL for plane updates
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (6 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 08/10] drm/i915: Consolidate top-level .update_plane() handlers Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-01 23:40 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2) Matt Roper
8 siblings, 0 replies; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
When disabling a plane, it is legal to pass crtc = NULL. Since planes
on Intel hardware are tied to a fixed CRTC, go ahead and set state->crtc
to the appropriate crtc in cases where it is passed to us as NULL.
In a future patch, we will start using the update handler for plane
disables, so this will help ensure we always have a non-NULL crtc
pointer to work with.
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f628a28..d13015c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11851,7 +11851,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int ret;
- state.base.crtc = crtc;
+ state.base.crtc = crtc ? crtc : plane->crtc;
state.base.fb = fb;
/* sample coordinates in 16.16 fixed point */
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2)
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
` (7 preceding siblings ...)
2014-12-01 23:40 ` [PATCH 09/10] drm/i915: Ensure state->crtc is non-NULL for plane updates Matt Roper
@ 2014-12-01 23:40 ` Matt Roper
2014-12-02 15:14 ` Ander Conselvan de Oliveira
8 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-01 23:40 UTC (permalink / raw)
To: intel-gfx
If we extend the commit_plane handlers for each plane type to be able to
handle fb=0, then we can easily implement plane disable via the
update_plane handler. The cursor plane already works this way, and this
is the direction we need to go to integrate with the atomic plane
handler. We can now kill off the type-specific disable functions, as
well as the redundant intel_plane_disable() (not to be confused with
intel_disable_plane()).
Note that prepare_plane_fb() only gets called as part of update_plane
when fb!=NULL (by design, to match the semantics of the atomic plane
helpers); this means that our commit_plane handlers need to handle the
frontbuffer tracking for the disable case, even though they don't handle
it for normal updates.
v2:
- Drop an unnecessary crtc access change (Ander)
- Change BUG_ON to WARN_ON (Ander/Daniel)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 101 +++++++++++++++--------------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 71 +++++++-----------------
3 files changed, 61 insertions(+), 113 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d13015c..d197bbb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
intel_plane = to_intel_plane(plane);
if (intel_plane->pipe == pipe)
- intel_plane_disable(&intel_plane->base);
+ plane->funcs->disable_plane(plane);
}
}
@@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
}
-static int
-intel_primary_plane_disable(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_crtc *intel_crtc;
-
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- intel_crtc = to_intel_crtc(plane->crtc);
-
- /*
- * Even though we checked plane->fb above, it's still possible that
- * the primary plane has been implicitly disabled because the crtc
- * coordinates given weren't visible, or because we detected
- * that it was 100% covered by a sprite plane. Or, the CRTC may be
- * off and we've set a fb, but haven't actually turned on the CRTC yet.
- * In either case, we need to unpin the FB and let the fb pointer get
- * updated, but otherwise we don't need to touch the hardware.
- */
- if (intel_crtc->primary_enabled) {
- intel_crtc_wait_for_pending_flips(plane->crtc);
- intel_disable_primary_hw_plane(plane, plane->crtc);
- }
-
- 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;
-
- return 0;
-}
-
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -11745,10 +11708,12 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return 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 (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;
+ }
}
return 0;
@@ -11763,12 +11728,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_device *dev = plane->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;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
+ enum pipe pipe = intel_plane->pipe;
- crtc->primary->fb = fb;
+ if (!fb) {
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_PRIMARY(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ plane->fb = fb;
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
@@ -11826,7 +11802,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
* explicitly disables the plane by passing fb=0
* because plane->fb still gets set and pinned.
*/
- intel_disable_primary_hw_plane(plane, crtc);
+ intel_disable_primary_hw_plane(plane, plane->crtc);
}
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
@@ -11897,6 +11873,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return 0;
}
+/**
+ * intel_disable_plane - disable a plane
+ * @plane: plane to disable
+ *
+ * General disable handler for all plane types.
+ */
+int
+intel_disable_plane(struct drm_plane *plane)
+{
+ if (!plane->fb)
+ return 0;
+
+ if(WARN_ON(!plane->crtc))
+ return -EINVAL;
+
+ return plane->funcs->update_plane(plane, plane->crtc, NULL,
+ 0, 0, 0, 0, 0, 0, 0, 0);
+}
+
/* Common destruction function for both primary and cursor planes */
static void intel_plane_destroy(struct drm_plane *plane)
{
@@ -11907,7 +11902,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
static const struct drm_plane_funcs intel_primary_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_primary_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property
};
@@ -11962,18 +11957,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
}
static int
-intel_cursor_plane_disable(struct drm_plane *plane)
-{
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- return plane->funcs->update_plane(plane, plane->crtc, NULL,
- 0, 0, 0, 0, 0, 0, 0, 0);
-}
-
-static int
intel_check_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
@@ -12097,7 +12080,7 @@ update:
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_cursor_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f7b6619..53b696e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
+int intel_disable_plane(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
@@ -1201,7 +1202,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,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bfd5270..bc5834b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
int max_scale, min_scale;
- int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ int pixel_size;
+
+ if (!fb) {
+ state->visible = false;
+ return 0;
+ }
/* Don't modify another pipe's plane */
if (intel_plane->pipe != intel_crtc->pipe) {
@@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (src_w < 3 || src_h < 3)
state->visible = false;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
width_bytes = ((src_x * pixel_size) & 63) +
src_w * pixel_size;
@@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
bool primary_enabled;
/*
+ * 'prepare' is never called when plane is being disabled, so we need
+ * to handle frontbuffer tracking here
+ */
+ if (!fb) {
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_SPRITE(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ /*
* If the sprite is completely covering the primary plane,
* we can disable the primary and save power.
*/
@@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static int
-intel_disable_plane(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct intel_crtc *intel_crtc;
- enum pipe pipe;
-
- if (!plane->fb)
- return 0;
-
- if (WARN_ON(!plane->crtc))
- return -EINVAL;
-
- intel_crtc = to_intel_crtc(plane->crtc);
- pipe = intel_crtc->pipe;
-
- if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = true;
-
- intel_plane->disable_plane(plane, plane->crtc);
-
- if (!primary_was_enabled && intel_crtc->primary_enabled)
- intel_post_enable_primary(plane->crtc);
- }
-
- if (intel_plane->obj) {
- if (intel_crtc->active)
- 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);
-
- intel_plane->obj = NULL;
- }
-
- return 0;
-}
-
static void intel_destroy_plane(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1478,14 +1451,6 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane->src_w, intel_plane->src_h);
}
-void intel_plane_disable(struct drm_plane *plane)
-{
- if (!plane->crtc || !plane->fb)
- return;
-
- intel_disable_plane(plane);
-}
-
static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2)
2014-12-01 23:40 ` [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2) Matt Roper
@ 2014-12-02 14:10 ` Ander Conselvan de Oliveira
2014-12-02 15:45 ` [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3) Matt Roper
0 siblings, 1 reply; 20+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-02 14:10 UTC (permalink / raw)
To: Matt Roper, intel-gfx
On 12/02/2014 01:40 AM, Matt Roper wrote:
> All plane update functions need to unpin the old framebuffer when
> flipping to a new one. Pull this logic into a separate function to ease
> the integration with atomic plane helpers.
>
> v2: Don't wait for vblank if we don't have an old fb to cleanup (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_sprite.c | 27 ++++++-----------
> 3 files changed, 51 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f48faaa..98e4fbf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
[...]
> @@ -12120,6 +12134,13 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
>
> intel_commit_cursor_plane(plane, &state);
>
> + if (fb != old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + if (old_fb)
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
We're still waiting for vblank when obj is NULL here.
[...]
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5d8c2e0..152a32d 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
[...]
> @@ -1390,6 +1373,14 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> }
>
> intel_commit_sprite_plane(plane, &state);
> +
> + if (fb != old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + if (old_fb)
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
And also here.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2)
2014-12-01 23:40 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2) Matt Roper
@ 2014-12-02 15:14 ` Ander Conselvan de Oliveira
2014-12-02 15:59 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v3) Matt Roper
0 siblings, 1 reply; 20+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-02 15:14 UTC (permalink / raw)
To: Matt Roper, intel-gfx
On 12/02/2014 01:40 AM, Matt Roper wrote:
> If we extend the commit_plane handlers for each plane type to be able to
> handle fb=0, then we can easily implement plane disable via the
> update_plane handler. The cursor plane already works this way, and this
> is the direction we need to go to integrate with the atomic plane
> handler. We can now kill off the type-specific disable functions, as
> well as the redundant intel_plane_disable() (not to be confused with
> intel_disable_plane()).
>
> Note that prepare_plane_fb() only gets called as part of update_plane
> when fb!=NULL (by design, to match the semantics of the atomic plane
> helpers); this means that our commit_plane handlers need to handle the
> frontbuffer tracking for the disable case, even though they don't handle
> it for normal updates.
>
> v2:
> - Drop an unnecessary crtc access change (Ander)
> - Change BUG_ON to WARN_ON (Ander/Daniel)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 101 +++++++++++++++--------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_sprite.c | 71 +++++++-----------------
> 3 files changed, 61 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d13015c..d197bbb 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> intel_plane = to_intel_plane(plane);
> if (intel_plane->pipe == pipe)
> - intel_plane_disable(&intel_plane->base);
> + plane->funcs->disable_plane(plane);
> }
> }
>
> @@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
> BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
> }
>
> -static int
> -intel_primary_plane_disable(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_crtc *intel_crtc;
> -
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> -
> - /*
> - * Even though we checked plane->fb above, it's still possible that
> - * the primary plane has been implicitly disabled because the crtc
> - * coordinates given weren't visible, or because we detected
> - * that it was 100% covered by a sprite plane. Or, the CRTC may be
> - * off and we've set a fb, but haven't actually turned on the CRTC yet.
> - * In either case, we need to unpin the FB and let the fb pointer get
> - * updated, but otherwise we don't need to touch the hardware.
> - */
> - if (intel_crtc->primary_enabled) {
> - intel_crtc_wait_for_pending_flips(plane->crtc);
> - intel_disable_primary_hw_plane(plane, plane->crtc);
> - }
> -
> - 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;
> -
> - return 0;
> -}
> -
> /**
> * intel_prepare_plane_fb - Prepare fb for usage on plane
> * @plane: drm plane to prepare for
> @@ -11745,10 +11708,12 @@ intel_check_primary_plane(struct drm_plane *plane,
> if (ret)
> return 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 (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;
> + }
Can we get here with a NULL plane->crtc for the disable case? At least intel_disable_plane() bails out in that case. But anyway, doesn't seem harmful.
> }
>
> return 0;
> @@ -11763,12 +11728,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
> struct drm_device *dev = plane->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;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_rect *src = &state->src;
> + enum pipe pipe = intel_plane->pipe;
>
> - crtc->primary->fb = fb;
> + if (!fb) {
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking here
> + */
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_PRIMARY(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + plane->fb = fb;
> crtc->x = src->x1 >> 16;
> crtc->y = src->y1 >> 16;
>
> @@ -11826,7 +11802,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
> * explicitly disables the plane by passing fb=0
> * because plane->fb still gets set and pinned.
> */
> - intel_disable_primary_hw_plane(plane, crtc);
> + intel_disable_primary_hw_plane(plane, plane->crtc);
Did you mean to remove this hunk? I'm fine either way, just wanted to
make sure I didn't miss anything, since it seems to me that plane->crtc
and crtc should always have the same value here.
> }
>
> intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
> @@ -11897,6 +11873,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> return 0;
> }
>
> +/**
> + * intel_disable_plane - disable a plane
> + * @plane: plane to disable
> + *
> + * General disable handler for all plane types.
> + */
> +int
> +intel_disable_plane(struct drm_plane *plane)
> +{
> + if (!plane->fb)
> + return 0;
> +
> + if(WARN_ON(!plane->crtc))
Missing space character. With this and the comments to patch 7 fixed, feel free to use
(for the series) Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Ander
> + return -EINVAL;
> +
> + return plane->funcs->update_plane(plane, plane->crtc, NULL,
> + 0, 0, 0, 0, 0, 0, 0, 0);
> +}
> +
> /* Common destruction function for both primary and cursor planes */
> static void intel_plane_destroy(struct drm_plane *plane)
> {
> @@ -11907,7 +11902,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
>
> static const struct drm_plane_funcs intel_primary_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_primary_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property
> };
> @@ -11962,18 +11957,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> }
>
> static int
> -intel_cursor_plane_disable(struct drm_plane *plane)
> -{
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - return plane->funcs->update_plane(plane, plane->crtc, NULL,
> - 0, 0, 0, 0, 0, 0, 0, 0);
> -}
> -
> -static int
> intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> @@ -12097,7 +12080,7 @@ update:
>
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_cursor_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f7b6619..53b696e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> unsigned int crtc_w, unsigned int crtc_h,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h);
> +int intel_disable_plane(struct drm_plane *plane);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> @@ -1201,7 +1202,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,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index bfd5270..bc5834b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> int max_scale, min_scale;
> - int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + int pixel_size;
> +
> + if (!fb) {
> + state->visible = false;
> + return 0;
> + }
>
> /* Don't modify another pipe's plane */
> if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (src_w < 3 || src_h < 3)
> state->visible = false;
>
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> width_bytes = ((src_x * pixel_size) & 63) +
> src_w * pixel_size;
>
> @@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> bool primary_enabled;
>
> /*
> + * 'prepare' is never called when plane is being disabled, so we need
> + * to handle frontbuffer tracking here
> + */
> + if (!fb) {
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_SPRITE(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + /*
> * If the sprite is completely covering the primary plane,
> * we can disable the primary and save power.
> */
> @@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> }
> }
>
> -static int
> -intel_disable_plane(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_plane *intel_plane = to_intel_plane(plane);
> - struct intel_crtc *intel_crtc;
> - enum pipe pipe;
> -
> - if (!plane->fb)
> - return 0;
> -
> - if (WARN_ON(!plane->crtc))
> - return -EINVAL;
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> - pipe = intel_crtc->pipe;
> -
> - if (intel_crtc->active) {
> - bool primary_was_enabled = intel_crtc->primary_enabled;
> -
> - intel_crtc->primary_enabled = true;
> -
> - intel_plane->disable_plane(plane, plane->crtc);
> -
> - if (!primary_was_enabled && intel_crtc->primary_enabled)
> - intel_post_enable_primary(plane->crtc);
> - }
> -
> - if (intel_plane->obj) {
> - if (intel_crtc->active)
> - 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);
> -
> - intel_plane->obj = NULL;
> - }
> -
> - return 0;
> -}
> -
> static void intel_destroy_plane(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -1478,14 +1451,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,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3)
2014-12-02 14:10 ` Ander Conselvan de Oliveira
@ 2014-12-02 15:45 ` Matt Roper
2014-12-03 9:52 ` Ander Conselvan de Oliveira
0 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-02 15:45 UTC (permalink / raw)
To: intel-gfx
All plane update functions need to unpin the old framebuffer when
flipping to a new one. Pull this logic into a separate function to ease
the integration with atomic plane helpers.
v2: Don't wait for vblank if we don't have an old fb to cleanup (Ander)
v3: Really don't wait for vblank if we don't have an old fb to cleanup.
Previous version only handled this for primary planes; we need the
same change on cursors/sprites too! (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++++++------------
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_sprite.c | 26 +++++-----------
3 files changed, 49 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f48faaa..86c4686 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11701,6 +11701,31 @@ intel_prepare_plane_fb(struct drm_plane *plane,
return ret;
}
+/**
+ * intel_cleanup_plane_fb - Cleans up an fb after plane use
+ * @plane: drm plane to clean up for
+ * @fb: old framebuffer that was on plane
+ *
+ * Cleans up a framebuffer that has just been removed from a plane.
+ */
+void
+intel_cleanup_plane_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);
+
+ if (WARN_ON(!obj))
+ return;
+
+ if (plane->type != DRM_PLANE_TYPE_CURSOR ||
+ !INTEL_INFO(dev)->cursor_needs_physical) {
+ mutex_lock(&dev->struct_mutex);
+ intel_unpin_fb_obj(obj);
+ mutex_unlock(&dev->struct_mutex);
+ }
+}
+
static int
intel_check_primary_plane(struct drm_plane *plane,
struct intel_plane_state *state)
@@ -11728,9 +11753,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;
@@ -11801,15 +11824,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
@@ -11819,6 +11833,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -11866,6 +11881,12 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
intel_commit_primary_plane(plane, &state);
+ if (fb != old_fb && old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
@@ -12048,14 +12069,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
else
addr = obj->phys_handle->busaddr;
- if (intel_crtc->cursor_bo) {
- if (!INTEL_INFO(dev)->cursor_needs_physical) {
- mutex_lock(&dev->struct_mutex);
- intel_unpin_fb_obj(intel_crtc->cursor_bo);
- mutex_unlock(&dev->struct_mutex);
- }
- }
-
intel_crtc->cursor_addr = addr;
intel_crtc->cursor_bo = obj;
update:
@@ -12080,6 +12093,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane_state state;
@@ -12120,6 +12134,12 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
intel_commit_cursor_plane(plane, &state);
+ if (fb != old_fb && old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a36f696..38cb553 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -924,6 +924,8 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
void intel_check_page_flip(struct drm_device *dev, int pipe);
int intel_prepare_plane_fb(struct drm_plane *plane,
struct drm_framebuffer *fb);
+void intel_cleanup_plane_fb(struct drm_plane *plane,
+ struct drm_framebuffer *fb);
/* shared dpll functions */
struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 5d8c2e0..031f95e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1268,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;
@@ -1326,23 +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
@@ -1352,6 +1334,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h)
{
+ struct drm_device *dev = plane->dev;
struct drm_framebuffer *old_fb = plane->fb;
struct intel_plane_state state;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -1390,6 +1373,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
}
intel_commit_sprite_plane(plane, &state);
+
+ if (fb != old_fb && old_fb) {
+ if (intel_crtc->active)
+ intel_wait_for_vblank(dev, intel_crtc->pipe);
+ intel_cleanup_plane_fb(plane, old_fb);
+ }
+
return 0;
}
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v3)
2014-12-02 15:14 ` Ander Conselvan de Oliveira
@ 2014-12-02 15:59 ` Matt Roper
2014-12-02 16:26 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v4) Matt Roper
0 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-02 15:59 UTC (permalink / raw)
To: intel-gfx
If we extend the commit_plane handlers for each plane type to be able to
handle fb=0, then we can easily implement plane disable via the
update_plane handler. The cursor plane already works this way, and this
is the direction we need to go to integrate with the atomic plane
handler. We can now kill off the type-specific disable functions, as
well as the redundant intel_plane_disable() (not to be confused with
intel_disable_plane()).
Note that prepare_plane_fb() only gets called as part of update_plane
when fb!=NULL (by design, to match the semantics of the atomic plane
helpers); this means that our commit_plane handlers need to handle the
frontbuffer tracking for the disable case, even though they don't handle
it for normal updates.
v2:
- Change BUG_ON to WARN_ON (Ander/Daniel)
v3:
- Drop unnecessary plane->crtc check since a previous patch to plane
update ensures that plane->crtc will always be non-NULL, even for
disable calls that might pass NULL from userspace. (Ander)
- Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 93 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 71 +++++++--------------------
3 files changed, 56 insertions(+), 110 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d13015c..0e80ed7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
intel_plane = to_intel_plane(plane);
if (intel_plane->pipe == pipe)
- intel_plane_disable(&intel_plane->base);
+ plane->funcs->disable_plane(plane);
}
}
@@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
}
-static int
-intel_primary_plane_disable(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_crtc *intel_crtc;
-
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- intel_crtc = to_intel_crtc(plane->crtc);
-
- /*
- * Even though we checked plane->fb above, it's still possible that
- * the primary plane has been implicitly disabled because the crtc
- * coordinates given weren't visible, or because we detected
- * that it was 100% covered by a sprite plane. Or, the CRTC may be
- * off and we've set a fb, but haven't actually turned on the CRTC yet.
- * In either case, we need to unpin the FB and let the fb pointer get
- * updated, but otherwise we don't need to touch the hardware.
- */
- if (intel_crtc->primary_enabled) {
- intel_crtc_wait_for_pending_flips(plane->crtc);
- intel_disable_primary_hw_plane(plane, plane->crtc);
- }
-
- 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;
-
- return 0;
-}
-
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -11745,8 +11708,8 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- intel_crtc_wait_for_pending_flips(crtc);
- if (intel_crtc_has_pending_flip(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;
}
@@ -11763,12 +11726,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_device *dev = plane->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;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
+ enum pipe pipe = intel_plane->pipe;
- crtc->primary->fb = fb;
+ if (!fb) {
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_PRIMARY(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ plane->fb = fb;
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
@@ -11897,6 +11871,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return 0;
}
+/**
+ * intel_disable_plane - disable a plane
+ * @plane: plane to disable
+ *
+ * General disable handler for all plane types.
+ */
+int
+intel_disable_plane(struct drm_plane *plane)
+{
+ if (!plane->fb)
+ return 0;
+
+ if(WARN_ON(!plane->crtc))
+ return -EINVAL;
+
+ return plane->funcs->update_plane(plane, plane->crtc, NULL,
+ 0, 0, 0, 0, 0, 0, 0, 0);
+}
+
/* Common destruction function for both primary and cursor planes */
static void intel_plane_destroy(struct drm_plane *plane)
{
@@ -11907,7 +11900,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
static const struct drm_plane_funcs intel_primary_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_primary_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property
};
@@ -11962,18 +11955,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
}
static int
-intel_cursor_plane_disable(struct drm_plane *plane)
-{
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- return plane->funcs->update_plane(plane, plane->crtc, NULL,
- 0, 0, 0, 0, 0, 0, 0, 0);
-}
-
-static int
intel_check_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
@@ -12097,7 +12078,7 @@ update:
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_cursor_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f7b6619..53b696e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
+int intel_disable_plane(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
@@ -1201,7 +1202,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,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bfd5270..bc5834b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
int max_scale, min_scale;
- int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ int pixel_size;
+
+ if (!fb) {
+ state->visible = false;
+ return 0;
+ }
/* Don't modify another pipe's plane */
if (intel_plane->pipe != intel_crtc->pipe) {
@@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (src_w < 3 || src_h < 3)
state->visible = false;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
width_bytes = ((src_x * pixel_size) & 63) +
src_w * pixel_size;
@@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
bool primary_enabled;
/*
+ * 'prepare' is never called when plane is being disabled, so we need
+ * to handle frontbuffer tracking here
+ */
+ if (!fb) {
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_SPRITE(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ /*
* If the sprite is completely covering the primary plane,
* we can disable the primary and save power.
*/
@@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static int
-intel_disable_plane(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct intel_crtc *intel_crtc;
- enum pipe pipe;
-
- if (!plane->fb)
- return 0;
-
- if (WARN_ON(!plane->crtc))
- return -EINVAL;
-
- intel_crtc = to_intel_crtc(plane->crtc);
- pipe = intel_crtc->pipe;
-
- if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = true;
-
- intel_plane->disable_plane(plane, plane->crtc);
-
- if (!primary_was_enabled && intel_crtc->primary_enabled)
- intel_post_enable_primary(plane->crtc);
- }
-
- if (intel_plane->obj) {
- if (intel_crtc->active)
- 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);
-
- intel_plane->obj = NULL;
- }
-
- return 0;
-}
-
static void intel_destroy_plane(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1478,14 +1451,6 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane->src_w, intel_plane->src_h);
}
-void intel_plane_disable(struct drm_plane *plane)
-{
- if (!plane->crtc || !plane->fb)
- return;
-
- intel_disable_plane(plane);
-}
-
static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v4)
2014-12-02 15:59 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v3) Matt Roper
@ 2014-12-02 16:26 ` Matt Roper
2014-12-03 9:53 ` Ander Conselvan de Oliveira
0 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-02 16:26 UTC (permalink / raw)
To: intel-gfx
If we extend the commit_plane handlers for each plane type to be able to
handle fb=0, then we can easily implement plane disable via the
update_plane handler. The cursor plane already works this way, and this
is the direction we need to go to integrate with the atomic plane
handler. We can now kill off the type-specific disable functions, as
well as the redundant intel_plane_disable() (not to be confused with
intel_disable_plane()).
Note that prepare_plane_fb() only gets called as part of update_plane
when fb!=NULL (by design, to match the semantics of the atomic plane
helpers); this means that our commit_plane handlers need to handle the
frontbuffer tracking for the disable case, even though they don't handle
it for normal updates.
v2:
- Change BUG_ON to WARN_ON (Ander/Daniel)
v3:
- Drop unnecessary plane->crtc check since a previous patch to plane
update ensures that plane->crtc will always be non-NULL, even for
disable calls that might pass NULL from userspace. (Ander)
- Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
v4:
- Fix missing whitespace (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 93 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 71 +++++++--------------------
3 files changed, 56 insertions(+), 110 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d13015c..eb422bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
intel_plane = to_intel_plane(plane);
if (intel_plane->pipe == pipe)
- intel_plane_disable(&intel_plane->base);
+ plane->funcs->disable_plane(plane);
}
}
@@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
}
-static int
-intel_primary_plane_disable(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_crtc *intel_crtc;
-
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- intel_crtc = to_intel_crtc(plane->crtc);
-
- /*
- * Even though we checked plane->fb above, it's still possible that
- * the primary plane has been implicitly disabled because the crtc
- * coordinates given weren't visible, or because we detected
- * that it was 100% covered by a sprite plane. Or, the CRTC may be
- * off and we've set a fb, but haven't actually turned on the CRTC yet.
- * In either case, we need to unpin the FB and let the fb pointer get
- * updated, but otherwise we don't need to touch the hardware.
- */
- if (intel_crtc->primary_enabled) {
- intel_crtc_wait_for_pending_flips(plane->crtc);
- intel_disable_primary_hw_plane(plane, plane->crtc);
- }
-
- 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;
-
- return 0;
-}
-
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -11745,8 +11708,8 @@ intel_check_primary_plane(struct drm_plane *plane,
if (ret)
return ret;
- intel_crtc_wait_for_pending_flips(crtc);
- if (intel_crtc_has_pending_flip(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;
}
@@ -11763,12 +11726,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_device *dev = plane->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;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
+ enum pipe pipe = intel_plane->pipe;
- crtc->primary->fb = fb;
+ if (!fb) {
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_PRIMARY(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ plane->fb = fb;
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
@@ -11897,6 +11871,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return 0;
}
+/**
+ * intel_disable_plane - disable a plane
+ * @plane: plane to disable
+ *
+ * General disable handler for all plane types.
+ */
+int
+intel_disable_plane(struct drm_plane *plane)
+{
+ if (!plane->fb)
+ return 0;
+
+ if (WARN_ON(!plane->crtc))
+ return -EINVAL;
+
+ return plane->funcs->update_plane(plane, plane->crtc, NULL,
+ 0, 0, 0, 0, 0, 0, 0, 0);
+}
+
/* Common destruction function for both primary and cursor planes */
static void intel_plane_destroy(struct drm_plane *plane)
{
@@ -11907,7 +11900,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
static const struct drm_plane_funcs intel_primary_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_primary_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property
};
@@ -11962,18 +11955,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
}
static int
-intel_cursor_plane_disable(struct drm_plane *plane)
-{
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- return plane->funcs->update_plane(plane, plane->crtc, NULL,
- 0, 0, 0, 0, 0, 0, 0, 0);
-}
-
-static int
intel_check_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
@@ -12097,7 +12078,7 @@ update:
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_cursor_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f7b6619..53b696e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
+int intel_disable_plane(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
@@ -1201,7 +1202,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,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bfd5270..bc5834b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
int max_scale, min_scale;
- int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ int pixel_size;
+
+ if (!fb) {
+ state->visible = false;
+ return 0;
+ }
/* Don't modify another pipe's plane */
if (intel_plane->pipe != intel_crtc->pipe) {
@@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (src_w < 3 || src_h < 3)
state->visible = false;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
width_bytes = ((src_x * pixel_size) & 63) +
src_w * pixel_size;
@@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
bool primary_enabled;
/*
+ * 'prepare' is never called when plane is being disabled, so we need
+ * to handle frontbuffer tracking here
+ */
+ if (!fb) {
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_SPRITE(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ /*
* If the sprite is completely covering the primary plane,
* we can disable the primary and save power.
*/
@@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static int
-intel_disable_plane(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct intel_crtc *intel_crtc;
- enum pipe pipe;
-
- if (!plane->fb)
- return 0;
-
- if (WARN_ON(!plane->crtc))
- return -EINVAL;
-
- intel_crtc = to_intel_crtc(plane->crtc);
- pipe = intel_crtc->pipe;
-
- if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = true;
-
- intel_plane->disable_plane(plane, plane->crtc);
-
- if (!primary_was_enabled && intel_crtc->primary_enabled)
- intel_post_enable_primary(plane->crtc);
- }
-
- if (intel_plane->obj) {
- if (intel_crtc->active)
- 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);
-
- intel_plane->obj = NULL;
- }
-
- return 0;
-}
-
static void intel_destroy_plane(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1478,14 +1451,6 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane->src_w, intel_plane->src_h);
}
-void intel_plane_disable(struct drm_plane *plane)
-{
- if (!plane->crtc || !plane->fb)
- return;
-
- intel_disable_plane(plane);
-}
-
static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3)
2014-12-02 15:45 ` [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3) Matt Roper
@ 2014-12-03 9:52 ` Ander Conselvan de Oliveira
0 siblings, 0 replies; 20+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-03 9:52 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
On 12/02/2014 05:45 PM, Matt Roper wrote:
> All plane update functions need to unpin the old framebuffer when
> flipping to a new one. Pull this logic into a separate function to ease
> the integration with atomic plane helpers.
>
> v2: Don't wait for vblank if we don't have an old fb to cleanup (Ander)
>
> v3: Really don't wait for vblank if we don't have an old fb to cleanup.
> Previous version only handled this for primary planes; we need the
> same change on cursors/sprites too! (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 58 ++++++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_sprite.c | 26 +++++-----------
> 3 files changed, 49 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f48faaa..86c4686 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11701,6 +11701,31 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> return ret;
> }
>
> +/**
> + * intel_cleanup_plane_fb - Cleans up an fb after plane use
> + * @plane: drm plane to clean up for
> + * @fb: old framebuffer that was on plane
> + *
> + * Cleans up a framebuffer that has just been removed from a plane.
> + */
> +void
> +intel_cleanup_plane_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);
> +
> + if (WARN_ON(!obj))
> + return;
> +
> + if (plane->type != DRM_PLANE_TYPE_CURSOR ||
> + !INTEL_INFO(dev)->cursor_needs_physical) {
> + mutex_lock(&dev->struct_mutex);
> + intel_unpin_fb_obj(obj);
> + mutex_unlock(&dev->struct_mutex);
> + }
> +}
> +
> static int
> intel_check_primary_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> @@ -11728,9 +11753,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;
>
> @@ -11801,15 +11824,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
> @@ -11819,6 +11833,7 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_plane_state state;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -11866,6 +11881,12 @@ intel_primary_plane_setplane(struct drm_plane *plane, struct drm_crtc *crtc,
>
> intel_commit_primary_plane(plane, &state);
>
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
> @@ -12048,14 +12069,6 @@ intel_commit_cursor_plane(struct drm_plane *plane,
> else
> addr = obj->phys_handle->busaddr;
>
> - if (intel_crtc->cursor_bo) {
> - if (!INTEL_INFO(dev)->cursor_needs_physical) {
> - mutex_lock(&dev->struct_mutex);
> - intel_unpin_fb_obj(intel_crtc->cursor_bo);
> - mutex_unlock(&dev->struct_mutex);
> - }
> - }
> -
> intel_crtc->cursor_addr = addr;
> intel_crtc->cursor_bo = obj;
> update:
> @@ -12080,6 +12093,7 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane_state state;
> @@ -12120,6 +12134,12 @@ intel_cursor_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
>
> intel_commit_cursor_plane(plane, &state);
>
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a36f696..38cb553 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -924,6 +924,8 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
> void intel_check_page_flip(struct drm_device *dev, int pipe);
> int intel_prepare_plane_fb(struct drm_plane *plane,
> struct drm_framebuffer *fb);
> +void intel_cleanup_plane_fb(struct drm_plane *plane,
> + struct drm_framebuffer *fb);
>
> /* shared dpll functions */
> struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 5d8c2e0..031f95e 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1268,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;
> @@ -1326,23 +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
> @@ -1352,6 +1334,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h)
> {
> + struct drm_device *dev = plane->dev;
> struct drm_framebuffer *old_fb = plane->fb;
> struct intel_plane_state state;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -1390,6 +1373,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> }
>
> intel_commit_sprite_plane(plane, &state);
> +
> + if (fb != old_fb && old_fb) {
> + if (intel_crtc->active)
> + intel_wait_for_vblank(dev, intel_crtc->pipe);
> + intel_cleanup_plane_fb(plane, old_fb);
> + }
> +
> return 0;
> }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v4)
2014-12-02 16:26 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v4) Matt Roper
@ 2014-12-03 9:53 ` Ander Conselvan de Oliveira
2014-12-04 18:27 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5) Matt Roper
0 siblings, 1 reply; 20+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-03 9:53 UTC (permalink / raw)
To: Matt Roper, intel-gfx
On 12/02/2014 06:26 PM, Matt Roper wrote:
> If we extend the commit_plane handlers for each plane type to be able to
> handle fb=0, then we can easily implement plane disable via the
> update_plane handler. The cursor plane already works this way, and this
> is the direction we need to go to integrate with the atomic plane
> handler. We can now kill off the type-specific disable functions, as
> well as the redundant intel_plane_disable() (not to be confused with
> intel_disable_plane()).
>
> Note that prepare_plane_fb() only gets called as part of update_plane
> when fb!=NULL (by design, to match the semantics of the atomic plane
> helpers); this means that our commit_plane handlers need to handle the
> frontbuffer tracking for the disable case, even though they don't handle
> it for normal updates.
>
> v2:
> - Change BUG_ON to WARN_ON (Ander/Daniel)
>
> v3:
> - Drop unnecessary plane->crtc check since a previous patch to plane
> update ensures that plane->crtc will always be non-NULL, even for
> disable calls that might pass NULL from userspace. (Ander)
> - Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
>
> v4:
> - Fix missing whitespace (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 93 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_sprite.c | 71 +++++++--------------------
> 3 files changed, 56 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d13015c..eb422bf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> intel_plane = to_intel_plane(plane);
> if (intel_plane->pipe == pipe)
> - intel_plane_disable(&intel_plane->base);
> + plane->funcs->disable_plane(plane);
> }
> }
>
> @@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
> BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
> }
>
> -static int
> -intel_primary_plane_disable(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_crtc *intel_crtc;
> -
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> -
> - /*
> - * Even though we checked plane->fb above, it's still possible that
> - * the primary plane has been implicitly disabled because the crtc
> - * coordinates given weren't visible, or because we detected
> - * that it was 100% covered by a sprite plane. Or, the CRTC may be
> - * off and we've set a fb, but haven't actually turned on the CRTC yet.
> - * In either case, we need to unpin the FB and let the fb pointer get
> - * updated, but otherwise we don't need to touch the hardware.
> - */
> - if (intel_crtc->primary_enabled) {
> - intel_crtc_wait_for_pending_flips(plane->crtc);
> - intel_disable_primary_hw_plane(plane, plane->crtc);
> - }
> -
> - 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;
> -
> - return 0;
> -}
> -
> /**
> * intel_prepare_plane_fb - Prepare fb for usage on plane
> * @plane: drm plane to prepare for
> @@ -11745,8 +11708,8 @@ intel_check_primary_plane(struct drm_plane *plane,
> if (ret)
> return ret;
>
> - intel_crtc_wait_for_pending_flips(crtc);
> - if (intel_crtc_has_pending_flip(crtc)) {
> + intel_crtc_wait_for_pending_flips(plane->crtc);
> + if (intel_crtc_has_pending_flip(plane->crtc)) {
So it turns out plane->crtc can be NULL here, when we enable the plane
for the first time. After drm_mode_set_config_internal() succeeds, it
sets primary->crtc and nothing else resets it to NULL. Now, in patch 9
you made sure state->base.crtc is always valid, so we could just leave
this hunk unchanged, or put back the 'if (plane->crtc)' check. Sorry for
the confusion.
Cheers,
Ander
> DRM_ERROR("pipe is still busy with an old pageflip\n");
> return -EBUSY;
> }
> @@ -11763,12 +11726,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
> struct drm_device *dev = plane->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;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_rect *src = &state->src;
> + enum pipe pipe = intel_plane->pipe;
>
> - crtc->primary->fb = fb;
> + if (!fb) {
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking here
> + */
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_PRIMARY(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + plane->fb = fb;
> crtc->x = src->x1 >> 16;
> crtc->y = src->y1 >> 16;
>
> @@ -11897,6 +11871,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> return 0;
> }
>
> +/**
> + * intel_disable_plane - disable a plane
> + * @plane: plane to disable
> + *
> + * General disable handler for all plane types.
> + */
> +int
> +intel_disable_plane(struct drm_plane *plane)
> +{
> + if (!plane->fb)
> + return 0;
> +
> + if (WARN_ON(!plane->crtc))
> + return -EINVAL;
> +
> + return plane->funcs->update_plane(plane, plane->crtc, NULL,
> + 0, 0, 0, 0, 0, 0, 0, 0);
> +}
> +
> /* Common destruction function for both primary and cursor planes */
> static void intel_plane_destroy(struct drm_plane *plane)
> {
> @@ -11907,7 +11900,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
>
> static const struct drm_plane_funcs intel_primary_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_primary_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property
> };
> @@ -11962,18 +11955,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> }
>
> static int
> -intel_cursor_plane_disable(struct drm_plane *plane)
> -{
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - return plane->funcs->update_plane(plane, plane->crtc, NULL,
> - 0, 0, 0, 0, 0, 0, 0, 0);
> -}
> -
> -static int
> intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> @@ -12097,7 +12078,7 @@ update:
>
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_cursor_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f7b6619..53b696e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> unsigned int crtc_w, unsigned int crtc_h,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h);
> +int intel_disable_plane(struct drm_plane *plane);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> @@ -1201,7 +1202,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,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index bfd5270..bc5834b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> int max_scale, min_scale;
> - int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + int pixel_size;
> +
> + if (!fb) {
> + state->visible = false;
> + return 0;
> + }
>
> /* Don't modify another pipe's plane */
> if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (src_w < 3 || src_h < 3)
> state->visible = false;
>
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> width_bytes = ((src_x * pixel_size) & 63) +
> src_w * pixel_size;
>
> @@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> bool primary_enabled;
>
> /*
> + * 'prepare' is never called when plane is being disabled, so we need
> + * to handle frontbuffer tracking here
> + */
> + if (!fb) {
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_SPRITE(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + /*
> * If the sprite is completely covering the primary plane,
> * we can disable the primary and save power.
> */
> @@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> }
> }
>
> -static int
> -intel_disable_plane(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_plane *intel_plane = to_intel_plane(plane);
> - struct intel_crtc *intel_crtc;
> - enum pipe pipe;
> -
> - if (!plane->fb)
> - return 0;
> -
> - if (WARN_ON(!plane->crtc))
> - return -EINVAL;
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> - pipe = intel_crtc->pipe;
> -
> - if (intel_crtc->active) {
> - bool primary_was_enabled = intel_crtc->primary_enabled;
> -
> - intel_crtc->primary_enabled = true;
> -
> - intel_plane->disable_plane(plane, plane->crtc);
> -
> - if (!primary_was_enabled && intel_crtc->primary_enabled)
> - intel_post_enable_primary(plane->crtc);
> - }
> -
> - if (intel_plane->obj) {
> - if (intel_crtc->active)
> - 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);
> -
> - intel_plane->obj = NULL;
> - }
> -
> - return 0;
> -}
> -
> static void intel_destroy_plane(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -1478,14 +1451,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,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5)
2014-12-03 9:53 ` Ander Conselvan de Oliveira
@ 2014-12-04 18:27 ` Matt Roper
2014-12-05 8:15 ` Ander Conselvan de Oliveira
0 siblings, 1 reply; 20+ messages in thread
From: Matt Roper @ 2014-12-04 18:27 UTC (permalink / raw)
To: intel-gfx
If we extend the commit_plane handlers for each plane type to be able to
handle fb=0, then we can easily implement plane disable via the
update_plane handler. The cursor plane already works this way, and this
is the direction we need to go to integrate with the atomic plane
handler. We can now kill off the type-specific disable functions, as
well as the redundant intel_plane_disable() (not to be confused with
intel_disable_plane()).
Note that prepare_plane_fb() only gets called as part of update_plane
when fb!=NULL (by design, to match the semantics of the atomic plane
helpers); this means that our commit_plane handlers need to handle the
frontbuffer tracking for the disable case, even though they don't handle
it for normal updates.
v2:
- Change BUG_ON to WARN_ON (Ander/Daniel)
v3:
- Drop unnecessary plane->crtc check since a previous patch to plane
update ensures that plane->crtc will always be non-NULL, even for
disable calls that might pass NULL from userspace. (Ander)
- Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
v4:
- Fix missing whitespace (Ander)
v5:
- Use state's crtc rather than plane's crtc in
intel_check_primary_plane(). plane->crtc could be NULL, but we've
already fixed up state->crtc to ensure it's non-NULL (even if
userspace passed it as NULL during a disable call). (Ander)
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 89 ++++++++++++++----------------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_sprite.c | 71 ++++++++--------------------
3 files changed, 54 insertions(+), 108 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d13015c..7e53994 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
intel_plane = to_intel_plane(plane);
if (intel_plane->pipe == pipe)
- intel_plane_disable(&intel_plane->base);
+ plane->funcs->disable_plane(plane);
}
}
@@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
}
-static int
-intel_primary_plane_disable(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_crtc *intel_crtc;
-
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- intel_crtc = to_intel_crtc(plane->crtc);
-
- /*
- * Even though we checked plane->fb above, it's still possible that
- * the primary plane has been implicitly disabled because the crtc
- * coordinates given weren't visible, or because we detected
- * that it was 100% covered by a sprite plane. Or, the CRTC may be
- * off and we've set a fb, but haven't actually turned on the CRTC yet.
- * In either case, we need to unpin the FB and let the fb pointer get
- * updated, but otherwise we don't need to touch the hardware.
- */
- if (intel_crtc->primary_enabled) {
- intel_crtc_wait_for_pending_flips(plane->crtc);
- intel_disable_primary_hw_plane(plane, plane->crtc);
- }
-
- 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;
-
- return 0;
-}
-
/**
* intel_prepare_plane_fb - Prepare fb for usage on plane
* @plane: drm plane to prepare for
@@ -11763,12 +11726,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_device *dev = plane->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;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
struct drm_rect *src = &state->src;
+ enum pipe pipe = intel_plane->pipe;
- crtc->primary->fb = fb;
+ if (!fb) {
+ /*
+ * 'prepare' is never called when plane is being disabled, so
+ * we need to handle frontbuffer tracking here
+ */
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_PRIMARY(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ plane->fb = fb;
crtc->x = src->x1 >> 16;
crtc->y = src->y1 >> 16;
@@ -11897,6 +11871,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
return 0;
}
+/**
+ * intel_disable_plane - disable a plane
+ * @plane: plane to disable
+ *
+ * General disable handler for all plane types.
+ */
+int
+intel_disable_plane(struct drm_plane *plane)
+{
+ if (!plane->fb)
+ return 0;
+
+ if (WARN_ON(!plane->crtc))
+ return -EINVAL;
+
+ return plane->funcs->update_plane(plane, plane->crtc, NULL,
+ 0, 0, 0, 0, 0, 0, 0, 0);
+}
+
/* Common destruction function for both primary and cursor planes */
static void intel_plane_destroy(struct drm_plane *plane)
{
@@ -11907,7 +11900,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
static const struct drm_plane_funcs intel_primary_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_primary_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property
};
@@ -11962,18 +11955,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
}
static int
-intel_cursor_plane_disable(struct drm_plane *plane)
-{
- if (!plane->fb)
- return 0;
-
- BUG_ON(!plane->crtc);
-
- return plane->funcs->update_plane(plane, plane->crtc, NULL,
- 0, 0, 0, 0, 0, 0, 0, 0);
-}
-
-static int
intel_check_cursor_plane(struct drm_plane *plane,
struct intel_plane_state *state)
{
@@ -12097,7 +12078,7 @@ update:
static const struct drm_plane_funcs intel_cursor_plane_funcs = {
.update_plane = intel_update_plane,
- .disable_plane = intel_cursor_plane_disable,
+ .disable_plane = intel_disable_plane,
.destroy = intel_plane_destroy,
.set_property = intel_plane_set_property,
};
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f7b6619..53b696e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
unsigned int crtc_w, unsigned int crtc_h,
uint32_t src_x, uint32_t src_y,
uint32_t src_w, uint32_t src_h);
+int intel_disable_plane(struct drm_plane *plane);
/* intel_dp_mst.c */
int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
@@ -1201,7 +1202,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,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bfd5270..bc5834b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
const struct drm_rect *clip = &state->clip;
int hscale, vscale;
int max_scale, min_scale;
- int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+ int pixel_size;
+
+ if (!fb) {
+ state->visible = false;
+ return 0;
+ }
/* Don't modify another pipe's plane */
if (intel_plane->pipe != intel_crtc->pipe) {
@@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (src_w < 3 || src_h < 3)
state->visible = false;
+ pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
width_bytes = ((src_x * pixel_size) & 63) +
src_w * pixel_size;
@@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
bool primary_enabled;
/*
+ * 'prepare' is never called when plane is being disabled, so we need
+ * to handle frontbuffer tracking here
+ */
+ if (!fb) {
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
+ INTEL_FRONTBUFFER_SPRITE(pipe));
+ mutex_unlock(&dev->struct_mutex);
+ }
+
+ /*
* If the sprite is completely covering the primary plane,
* we can disable the primary and save power.
*/
@@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
}
}
-static int
-intel_disable_plane(struct drm_plane *plane)
-{
- struct drm_device *dev = plane->dev;
- struct intel_plane *intel_plane = to_intel_plane(plane);
- struct intel_crtc *intel_crtc;
- enum pipe pipe;
-
- if (!plane->fb)
- return 0;
-
- if (WARN_ON(!plane->crtc))
- return -EINVAL;
-
- intel_crtc = to_intel_crtc(plane->crtc);
- pipe = intel_crtc->pipe;
-
- if (intel_crtc->active) {
- bool primary_was_enabled = intel_crtc->primary_enabled;
-
- intel_crtc->primary_enabled = true;
-
- intel_plane->disable_plane(plane, plane->crtc);
-
- if (!primary_was_enabled && intel_crtc->primary_enabled)
- intel_post_enable_primary(plane->crtc);
- }
-
- if (intel_plane->obj) {
- if (intel_crtc->active)
- 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);
-
- intel_plane->obj = NULL;
- }
-
- return 0;
-}
-
static void intel_destroy_plane(struct drm_plane *plane)
{
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -1478,14 +1451,6 @@ int intel_plane_restore(struct drm_plane *plane)
intel_plane->src_w, intel_plane->src_h);
}
-void intel_plane_disable(struct drm_plane *plane)
-{
- if (!plane->crtc || !plane->fb)
- return;
-
- intel_disable_plane(plane);
-}
-
static const struct drm_plane_funcs intel_plane_funcs = {
.update_plane = intel_update_plane,
.disable_plane = intel_disable_plane,
--
1.8.5.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5)
2014-12-04 18:27 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5) Matt Roper
@ 2014-12-05 8:15 ` Ander Conselvan de Oliveira
2014-12-05 20:31 ` Daniel Vetter
0 siblings, 1 reply; 20+ messages in thread
From: Ander Conselvan de Oliveira @ 2014-12-05 8:15 UTC (permalink / raw)
To: Matt Roper, intel-gfx
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
On 12/04/2014 08:27 PM, Matt Roper wrote:
> If we extend the commit_plane handlers for each plane type to be able to
> handle fb=0, then we can easily implement plane disable via the
> update_plane handler. The cursor plane already works this way, and this
> is the direction we need to go to integrate with the atomic plane
> handler. We can now kill off the type-specific disable functions, as
> well as the redundant intel_plane_disable() (not to be confused with
> intel_disable_plane()).
>
> Note that prepare_plane_fb() only gets called as part of update_plane
> when fb!=NULL (by design, to match the semantics of the atomic plane
> helpers); this means that our commit_plane handlers need to handle the
> frontbuffer tracking for the disable case, even though they don't handle
> it for normal updates.
>
> v2:
> - Change BUG_ON to WARN_ON (Ander/Daniel)
>
> v3:
> - Drop unnecessary plane->crtc check since a previous patch to plane
> update ensures that plane->crtc will always be non-NULL, even for
> disable calls that might pass NULL from userspace. (Ander)
> - Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
>
> v4:
> - Fix missing whitespace (Ander)
>
> v5:
> - Use state's crtc rather than plane's crtc in
> intel_check_primary_plane(). plane->crtc could be NULL, but we've
> already fixed up state->crtc to ensure it's non-NULL (even if
> userspace passed it as NULL during a disable call). (Ander)
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 89 ++++++++++++++----------------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_sprite.c | 71 ++++++++--------------------
> 3 files changed, 54 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d13015c..7e53994 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4060,7 +4060,7 @@ static void intel_disable_planes(struct drm_crtc *crtc)
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> intel_plane = to_intel_plane(plane);
> if (intel_plane->pipe == pipe)
> - intel_plane_disable(&intel_plane->base);
> + plane->funcs->disable_plane(plane);
> }
> }
>
> @@ -11605,43 +11605,6 @@ static void intel_shared_dpll_init(struct drm_device *dev)
> BUG_ON(dev_priv->num_shared_dpll > I915_NUM_PLLS);
> }
>
> -static int
> -intel_primary_plane_disable(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_crtc *intel_crtc;
> -
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> -
> - /*
> - * Even though we checked plane->fb above, it's still possible that
> - * the primary plane has been implicitly disabled because the crtc
> - * coordinates given weren't visible, or because we detected
> - * that it was 100% covered by a sprite plane. Or, the CRTC may be
> - * off and we've set a fb, but haven't actually turned on the CRTC yet.
> - * In either case, we need to unpin the FB and let the fb pointer get
> - * updated, but otherwise we don't need to touch the hardware.
> - */
> - if (intel_crtc->primary_enabled) {
> - intel_crtc_wait_for_pending_flips(plane->crtc);
> - intel_disable_primary_hw_plane(plane, plane->crtc);
> - }
> -
> - 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;
> -
> - return 0;
> -}
> -
> /**
> * intel_prepare_plane_fb - Prepare fb for usage on plane
> * @plane: drm plane to prepare for
> @@ -11763,12 +11726,23 @@ intel_commit_primary_plane(struct drm_plane *plane,
> struct drm_device *dev = plane->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;
> struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> struct drm_rect *src = &state->src;
> + enum pipe pipe = intel_plane->pipe;
>
> - crtc->primary->fb = fb;
> + if (!fb) {
> + /*
> + * 'prepare' is never called when plane is being disabled, so
> + * we need to handle frontbuffer tracking here
> + */
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_PRIMARY(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + plane->fb = fb;
> crtc->x = src->x1 >> 16;
> crtc->y = src->y1 >> 16;
>
> @@ -11897,6 +11871,25 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> return 0;
> }
>
> +/**
> + * intel_disable_plane - disable a plane
> + * @plane: plane to disable
> + *
> + * General disable handler for all plane types.
> + */
> +int
> +intel_disable_plane(struct drm_plane *plane)
> +{
> + if (!plane->fb)
> + return 0;
> +
> + if (WARN_ON(!plane->crtc))
> + return -EINVAL;
> +
> + return plane->funcs->update_plane(plane, plane->crtc, NULL,
> + 0, 0, 0, 0, 0, 0, 0, 0);
> +}
> +
> /* Common destruction function for both primary and cursor planes */
> static void intel_plane_destroy(struct drm_plane *plane)
> {
> @@ -11907,7 +11900,7 @@ static void intel_plane_destroy(struct drm_plane *plane)
>
> static const struct drm_plane_funcs intel_primary_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_primary_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property
> };
> @@ -11962,18 +11955,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
> }
>
> static int
> -intel_cursor_plane_disable(struct drm_plane *plane)
> -{
> - if (!plane->fb)
> - return 0;
> -
> - BUG_ON(!plane->crtc);
> -
> - return plane->funcs->update_plane(plane, plane->crtc, NULL,
> - 0, 0, 0, 0, 0, 0, 0, 0);
> -}
> -
> -static int
> intel_check_cursor_plane(struct drm_plane *plane,
> struct intel_plane_state *state)
> {
> @@ -12097,7 +12078,7 @@ update:
>
> static const struct drm_plane_funcs intel_cursor_plane_funcs = {
> .update_plane = intel_update_plane,
> - .disable_plane = intel_cursor_plane_disable,
> + .disable_plane = intel_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = intel_plane_set_property,
> };
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f7b6619..53b696e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1020,6 +1020,7 @@ int intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> unsigned int crtc_w, unsigned int crtc_h,
> uint32_t src_x, uint32_t src_y,
> uint32_t src_w, uint32_t src_h);
> +int intel_disable_plane(struct drm_plane *plane);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
> @@ -1201,7 +1202,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,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index bfd5270..bc5834b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1109,7 +1109,12 @@ intel_check_sprite_plane(struct drm_plane *plane,
> const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> int max_scale, min_scale;
> - int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> + int pixel_size;
> +
> + if (!fb) {
> + state->visible = false;
> + return 0;
> + }
>
> /* Don't modify another pipe's plane */
> if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -1232,6 +1237,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
> if (src_w < 3 || src_h < 3)
> state->visible = false;
>
> + pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> width_bytes = ((src_x * pixel_size) & 63) +
> src_w * pixel_size;
>
> @@ -1276,6 +1282,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> bool primary_enabled;
>
> /*
> + * 'prepare' is never called when plane is being disabled, so we need
> + * to handle frontbuffer tracking here
> + */
> + if (!fb) {
> + mutex_lock(&dev->struct_mutex);
> + i915_gem_track_fb(intel_fb_obj(plane->fb), NULL,
> + INTEL_FRONTBUFFER_SPRITE(pipe));
> + mutex_unlock(&dev->struct_mutex);
> + }
> +
> + /*
> * If the sprite is completely covering the primary plane,
> * we can disable the primary and save power.
> */
> @@ -1327,50 +1344,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
> }
> }
>
> -static int
> -intel_disable_plane(struct drm_plane *plane)
> -{
> - struct drm_device *dev = plane->dev;
> - struct intel_plane *intel_plane = to_intel_plane(plane);
> - struct intel_crtc *intel_crtc;
> - enum pipe pipe;
> -
> - if (!plane->fb)
> - return 0;
> -
> - if (WARN_ON(!plane->crtc))
> - return -EINVAL;
> -
> - intel_crtc = to_intel_crtc(plane->crtc);
> - pipe = intel_crtc->pipe;
> -
> - if (intel_crtc->active) {
> - bool primary_was_enabled = intel_crtc->primary_enabled;
> -
> - intel_crtc->primary_enabled = true;
> -
> - intel_plane->disable_plane(plane, plane->crtc);
> -
> - if (!primary_was_enabled && intel_crtc->primary_enabled)
> - intel_post_enable_primary(plane->crtc);
> - }
> -
> - if (intel_plane->obj) {
> - if (intel_crtc->active)
> - 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);
> -
> - intel_plane->obj = NULL;
> - }
> -
> - return 0;
> -}
> -
> static void intel_destroy_plane(struct drm_plane *plane)
> {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> @@ -1478,14 +1451,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,
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5)
2014-12-05 8:15 ` Ander Conselvan de Oliveira
@ 2014-12-05 20:31 ` Daniel Vetter
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Vetter @ 2014-12-05 20:31 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
On Fri, Dec 05, 2014 at 10:15:07AM +0200, Ander Conselvan de Oliveira wrote:
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> On 12/04/2014 08:27 PM, Matt Roper wrote:
> > If we extend the commit_plane handlers for each plane type to be able to
> > handle fb=0, then we can easily implement plane disable via the
> > update_plane handler. The cursor plane already works this way, and this
> > is the direction we need to go to integrate with the atomic plane
> > handler. We can now kill off the type-specific disable functions, as
> > well as the redundant intel_plane_disable() (not to be confused with
> > intel_disable_plane()).
> >
> > Note that prepare_plane_fb() only gets called as part of update_plane
> > when fb!=NULL (by design, to match the semantics of the atomic plane
> > helpers); this means that our commit_plane handlers need to handle the
> > frontbuffer tracking for the disable case, even though they don't handle
> > it for normal updates.
> >
> > v2:
> > - Change BUG_ON to WARN_ON (Ander/Daniel)
> >
> > v3:
> > - Drop unnecessary plane->crtc check since a previous patch to plane
> > update ensures that plane->crtc will always be non-NULL, even for
> > disable calls that might pass NULL from userspace. (Ander)
> > - Drop a s/crtc/plane->crtc/ hunk that was unnecessary. (Ander)
> >
> > v4:
> > - Fix missing whitespace (Ander)
> >
> > v5:
> > - Use state's crtc rather than plane's crtc in
> > intel_check_primary_plane(). plane->crtc could be NULL, but we've
> > already fixed up state->crtc to ensure it's non-NULL (even if
> > userspace passed it as NULL during a disable call). (Ander)
> >
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Ok, merged all 10 patches to dinq, thanks. There was a minor conflict in
patch 8 due to the slight changes in patch 7, but nothing big really.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-12-05 20:31 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-01 23:40 [PATCH 01/10] drm: add helper to get crtc timings (v5) Matt Roper
2014-12-01 23:40 ` [PATCH 02/10] drm/i915: remove intel_crtc_cursor_set_obj() (v5) Matt Roper
2014-12-01 23:40 ` [PATCH 03/10] drm/i915: remove intel_pipe_set_base() (v4) Matt Roper
2014-12-01 23:40 ` [PATCH 04/10] drm/i915: Introduce intel_prepare_cursor_plane() (v2) Matt Roper
2014-12-01 23:40 ` [PATCH 05/10] drm/i915: Make intel_plane_state subclass drm_plane_state Matt Roper
2014-12-01 23:40 ` [PATCH 06/10] drm/i915: Consolidate plane 'prepare' functions (v2) Matt Roper
2014-12-01 23:40 ` [PATCH 07/10] drm/i915: Consolidate plane 'cleanup' operations (v2) Matt Roper
2014-12-02 14:10 ` Ander Conselvan de Oliveira
2014-12-02 15:45 ` [PATCH 7/7] drm/i915: Consolidate plane 'cleanup' operations (v3) Matt Roper
2014-12-03 9:52 ` Ander Conselvan de Oliveira
2014-12-01 23:40 ` [PATCH 08/10] drm/i915: Consolidate top-level .update_plane() handlers Matt Roper
2014-12-01 23:40 ` [PATCH 09/10] drm/i915: Ensure state->crtc is non-NULL for plane updates Matt Roper
2014-12-01 23:40 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v2) Matt Roper
2014-12-02 15:14 ` Ander Conselvan de Oliveira
2014-12-02 15:59 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v3) Matt Roper
2014-12-02 16:26 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v4) Matt Roper
2014-12-03 9:53 ` Ander Conselvan de Oliveira
2014-12-04 18:27 ` [PATCH 10/10] drm/i915: Make all plane disables use 'update_plane' (v5) Matt Roper
2014-12-05 8:15 ` Ander Conselvan de Oliveira
2014-12-05 20:31 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.