From b079bd17e301708da044b4ea114f44c4aa47160c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 25 Oct 2016 18:58:02 +0300 Subject: [PATCH] drm/i915: Bail if plane/crtc init fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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ä Link: http://patchwork.freedesktop.org/patch/msgid/1477411083-19255-4-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 103 +++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_sprite.c | 8 +-- 5 files changed, 76 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 839ce2ae38fa..6a99544c98d3 100644 --- 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 42a499681966..51360d199263 100644 --- 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 b9692f639a9f..740c68896672 100644 --- 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 @@ static void sanitize_watermarks(struct drm_device *dev) 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 2616d92f9fee..46233356ed24 100644 --- 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 ae1e3d47951b..41ae7f562eec 100644 --- 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); } -- GitLab