drm/i915: Make all plane disables use 'update_plane' (v5)

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>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4f24cce0fe385f9d4523d359bf18ce624390fc47..c17419bb8bc09ec21f705b9946f4a2e51f637f5d 100644 (file)
--- 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);
        }
 }
 
@@ -11652,43 +11652,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
@@ -11810,12 +11773,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;
 
@@ -11944,6 +11918,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)
 {
@@ -11954,7 +11947,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
 };
@@ -12008,18 +12001,6 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
        return &primary->base;
 }
 
-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)
@@ -12144,7 +12125,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 f7b661969ad09043d14676998dc5c19aeb1156f3..53b696e841cd4e7201750f244be3afd5f9f6264a 100644 (file)
--- 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 bfd5270bdbd5ffed8da7214e92c108c6e15feb19..bc5834bba2ae8620e79c9279df333a48158702ef 100644 (file)
--- 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;
 
@@ -1275,6 +1281,17 @@ intel_commit_sprite_plane(struct drm_plane *plane,
        const struct drm_rect *clip = &state->clip;
        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,