drm/i915: Bail if plane/crtc init fails

Due to the plane->index not getting readjusted in drm_plane_cleanup(),
we can't continue initialization of some plane/crtc init fails.
Well, we sort of could I suppose if we left all initialized planes on
the list, but that would expose those planes to userspace as well.

But for crtcs the situation is even worse since we assume that
pipe==crtc index occasionally, so we can't really deal with a partially
initialize set of crtcs.

So seems safest to just abort the entire thing if anything goes wrong.
All the failure paths here are kmalloc()s anyway, so it seems unlikely
we'd get very far if these start failing.

v2: Add (enum plane) case to silence gcc

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1477411083-19255-4-git-send-email-ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 839ce2ae38fa90df46045d7d05070ccec2b4ead8..6a99544c98d3236213ff04eaf1f37b374e8d12ab 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -595,7 +595,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 
        /* Important: The output setup functions called by modeset_init need
         * working irqs for e.g. gmbus and dp aux transfers. */
-       intel_modeset_init(dev);
+       ret = intel_modeset_init(dev);
+       if (ret)
+               goto cleanup_irq;
 
        intel_guc_init(dev);
 

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 42a49968196666b7f737c49a256871c07deadfe0..51360d199263a6b23dcff570f96bc12a89466f56 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3799,7 +3799,7 @@ void intel_device_info_dump(struct drm_i915_private *dev_priv);
 
 /* modesetting */
 extern void intel_modeset_init_hw(struct drm_device *dev);
-extern void intel_modeset_init(struct drm_device *dev);
+extern int intel_modeset_init(struct drm_device *dev);
 extern void intel_modeset_gem_init(struct drm_device *dev);
 extern void intel_modeset_cleanup(struct drm_device *dev);
 extern int intel_connector_register(struct drm_connector *);

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b9692f639a9f704225a18139ab2495166fe7e13f..740c688966726247290b7c5eef4965fbedb75b73 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14944,9 +14944,6 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
  */
 void intel_plane_destroy(struct drm_plane *plane)
 {
-       if (!plane)
-               return;
-
        drm_plane_cleanup(plane);
        kfree(to_intel_plane(plane));
 }
@@ -14960,11 +14957,10 @@ const struct drm_plane_funcs intel_plane_funcs = {
        .atomic_set_property = intel_plane_atomic_set_property,
        .atomic_duplicate_state = intel_plane_duplicate_state,
        .atomic_destroy_state = intel_plane_destroy_state,
-
 };
 
-static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
-                                                   int pipe)
+static struct intel_plane *
+intel_primary_plane_create(struct drm_device *dev, enum pipe pipe)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct intel_plane *primary = NULL;
@@ -14975,12 +14971,17 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
        int ret;
 
        primary = kzalloc(sizeof(*primary), GFP_KERNEL);
-       if (!primary)
+       if (!primary) {
+               ret = -ENOMEM;
                goto fail;
+       }
 
        state = intel_create_plane_state(&primary->base);
-       if (!state)
+       if (!state) {
+               ret = -ENOMEM;
                goto fail;
+       }
+
        primary->base.state = &state->base;
 
        primary->can_scale = false;
@@ -15061,13 +15062,13 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 
        drm_plane_helper_add(&primary->base, &intel_plane_helper_funcs);
 
-       return &primary->base;
+       return primary;
 
 fail:
        kfree(state);
        kfree(primary);
 
-       return NULL;
+       return ERR_PTR(ret);
 }
 
 static int
@@ -15163,8 +15164,8 @@ intel_update_cursor_plane(struct drm_plane *plane,
        intel_crtc_update_cursor(crtc, state);
 }
 
-static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
-                                                  int pipe)
+static struct intel_plane *
+intel_cursor_plane_create(struct drm_device *dev, enum pipe pipe)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct intel_plane *cursor = NULL;
@@ -15172,12 +15173,17 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
        int ret;
 
        cursor = kzalloc(sizeof(*cursor), GFP_KERNEL);
-       if (!cursor)
+       if (!cursor) {
+               ret = -ENOMEM;
                goto fail;
+       }
 
        state = intel_create_plane_state(&cursor->base);
-       if (!state)
+       if (!state) {
+               ret = -ENOMEM;
                goto fail;
+       }
+
        cursor->base.state = &state->base;
 
        cursor->can_scale = false;
@@ -15209,13 +15215,13 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 
        drm_plane_helper_add(&cursor->base, &intel_plane_helper_funcs);
 
-       return &cursor->base;
+       return cursor;
 
 fail:
        kfree(state);
        kfree(cursor);
 
-       return NULL;
+       return ERR_PTR(ret);
 }
 
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
@@ -15234,22 +15240,24 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
        scaler_state->scaler_id = -1;
 }
 
-static void intel_crtc_init(struct drm_device *dev, int pipe)
+static int intel_crtc_init(struct drm_device *dev, enum pipe pipe)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct intel_crtc *intel_crtc;
        struct intel_crtc_state *crtc_state = NULL;
-       struct drm_plane *primary = NULL;
-       struct drm_plane *cursor = NULL;
+       struct intel_plane *primary = NULL;
+       struct intel_plane *cursor = NULL;
        int sprite, ret;
 
        intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
-       if (intel_crtc == NULL)
-               return;
+       if (!intel_crtc)
+               return -ENOMEM;
 
        crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
-       if (!crtc_state)
+       if (!crtc_state) {
+               ret = -ENOMEM;
                goto fail;
+       }
        intel_crtc->config = crtc_state;
        intel_crtc->base.state = &crtc_state->base;
        crtc_state->base.crtc = &intel_crtc->base;
@@ -15265,22 +15273,30 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
        }
 
        primary = intel_primary_plane_create(dev, pipe);
-       if (!primary)
+       if (IS_ERR(primary)) {
+               ret = PTR_ERR(primary);
                goto fail;
+       }
 
        for_each_sprite(dev_priv, pipe, sprite) {
-               ret = intel_plane_init(dev, pipe, sprite);
-               if (ret)
-                       DRM_DEBUG_KMS("pipe %c sprite %c init failed: %d\n",
-                                     pipe_name(pipe), sprite_name(pipe, sprite), ret);
+               struct intel_plane *plane;
+
+               plane = intel_sprite_plane_create(dev, pipe, sprite);
+               if (!plane) {
+                       ret = PTR_ERR(plane);
+                       goto fail;
+               }
        }
 
        cursor = intel_cursor_plane_create(dev, pipe);
-       if (!cursor)
+       if (!cursor) {
+               ret = PTR_ERR(cursor);
                goto fail;
+       }
 
-       ret = drm_crtc_init_with_planes(dev, &intel_crtc->base, primary,
-                                       cursor, &intel_crtc_funcs,
+       ret = drm_crtc_init_with_planes(dev, &intel_crtc->base,
+                                       &primary->base, &cursor->base,
+                                       &intel_crtc_funcs,
                                        "pipe %c", pipe_name(pipe));
        if (ret)
                goto fail;
@@ -15290,7 +15306,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
         * is hooked to pipe B. Hence we want plane A feeding pipe B.
         */
        intel_crtc->pipe = pipe;
-       intel_crtc->plane = pipe;
+       intel_crtc->plane = (enum plane) pipe;
        if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4) {
                DRM_DEBUG_KMS("swapping pipes & planes for FBC\n");
                intel_crtc->plane = !pipe;
@@ -15312,13 +15328,18 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
        intel_color_init(&intel_crtc->base);
 
        WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
-       return;
+
+       return 0;
 
 fail:
-       intel_plane_destroy(primary);
-       intel_plane_destroy(cursor);
+       /*
+        * drm_mode_config_cleanup() will free up any
+        * crtcs/planes already initialized.
+        */
        kfree(crtc_state);
        kfree(intel_crtc);
+
+       return ret;
 }
 
 enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
@@ -16395,7 +16416,7 @@ fail:
        drm_modeset_acquire_fini(&ctx);
 }
 
-void intel_modeset_init(struct drm_device *dev)
+int intel_modeset_init(struct drm_device *dev)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct i915_ggtt *ggtt = &dev_priv->ggtt;
@@ -16419,7 +16440,7 @@ void intel_modeset_init(struct drm_device *dev)
        intel_init_pm(dev);
 
        if (INTEL_INFO(dev)->num_pipes == 0)
-               return;
+               return 0;
 
        /*
         * There may be no VBT; and if the BIOS enabled SSC we can
@@ -16468,7 +16489,13 @@ void intel_modeset_init(struct drm_device *dev)
                      INTEL_INFO(dev)->num_pipes > 1 ? "s" : "");
 
        for_each_pipe(dev_priv, pipe) {
-               intel_crtc_init(dev, pipe);
+               int ret;
+
+               ret = intel_crtc_init(dev, pipe);
+               if (ret) {
+                       drm_mode_config_cleanup(dev);
+                       return ret;
+               }
        }
 
        intel_update_czclk(dev_priv);
@@ -16516,6 +16543,8 @@ void intel_modeset_init(struct drm_device *dev)
         * since the watermark calculation done here will use pstate->fb.
         */
        sanitize_watermarks(dev);
+
+       return 0;
 }
 
 static void intel_enable_pipe_a(struct drm_device *dev)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2616d92f9fee2867f9776b55e9fb822b892f350d..46233356ed24c1eaa00c7bbe918e9b49ee2940f2 100644 (file)
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1778,7 +1778,8 @@ bool intel_sdvo_init(struct drm_device *dev,
 /* intel_sprite.c */
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
                             int usecs);
-int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
+struct intel_plane *intel_sprite_plane_create(struct drm_device *dev,
+                                             enum pipe pipe, int plane);
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
                              struct drm_file *file_priv);
 void intel_pipe_update_start(struct intel_crtc *crtc);

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index ae1e3d47951b254a73df119da716813ea6699e37..41ae7f562eec3a6e6f758909bacef456bb024d71 100644 (file)
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1042,8 +1042,8 @@ static uint32_t skl_plane_formats[] = {
        DRM_FORMAT_VYUY,
 };
 
-int
-intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
+struct intel_plane *
+intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane)
 {
        struct drm_i915_private *dev_priv = to_i915(dev);
        struct intel_plane *intel_plane = NULL;
@@ -1160,11 +1160,11 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 
        drm_plane_helper_add(&intel_plane->base, &intel_plane_helper_funcs);
 
-       return 0;
+       return intel_plane;
 
 fail:
        kfree(state);
        kfree(intel_plane);
 
-       return ret;
+       return ERR_PTR(ret);
 }