* [PATCH 0/4] drm/i915: Abort if crtc/plane init fails
@ 2016-10-25 15:57 ville.syrjala
2016-10-25 15:58 ` [PATCH 1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk ville.syrjala
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: ville.syrjala @ 2016-10-25 15:57 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
I recently realized that we can't really continue with loading the driver
if we fail to initialize some of the crtcs or planes. So this series makes
us fail the load in those cases. The failures would be due to kmalloc()
failing anyway, so doesn't seem too drastic to abort entirely in that case.
I also reorder things so that we'll initialize the planes in an order that
matches the new rules for handling zpos conflicts between the planes. We don't
expose the zpos property yet, but I have some preliminary patches for that
as well sitting around in a branch. Actually only VLV, CHV and pre-g4x can
dynamically adjust the zpos of the planes, for the rest it's entirely fixed.
And finally I do a bit of house cleaning in the sprite init code.
Entire series available here:
git://github.com/vsyrjala/linux.git plane_init_order
Ville Syrjälä (4):
drm/i915: Don't try to initialize sprite planes on pre-ilk
drm/i915: Initialize planes in a reasonable order
drm/i915: Bail if plane/crtc init fails
drm/i915: Reorganize sprite init
drivers/gpu/drm/i915/i915_drv.c | 4 +-
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_device_info.c | 5 +-
drivers/gpu/drm/i915/intel_display.c | 107 ++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_drv.h | 3 +-
drivers/gpu/drm/i915/intel_sprite.c | 81 ++++++++++-------------
6 files changed, 112 insertions(+), 90 deletions(-)
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala @ 2016-10-25 15:58 ` ville.syrjala 2016-10-25 15:58 ` [PATCH 2/4] drm/i915: Initialize planes in a reasonable order ville.syrjala ` (3 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: ville.syrjala @ 2016-10-25 15:58 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> We don't currently implement support for sprite planes on pre-ilk platforms, so let's leave num_sprites at 0 so that we don't get spurious errors during driver init. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_device_info.c | 5 +++-- drivers/gpu/drm/i915/intel_sprite.c | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index d6a8f11813d5..185e3bbc9ec9 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -282,12 +282,13 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) info->num_sprites[PIPE_A] = 2; info->num_sprites[PIPE_B] = 2; info->num_sprites[PIPE_C] = 1; - } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 2; - else + } else if (INTEL_GEN(dev_priv) >= 5) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 1; + } if (i915.disable_display) { DRM_INFO("Display disabled (module parameter)\n"); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 43d0350856e7..ae1e3d47951b 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1054,9 +1054,6 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane) int num_plane_formats; int ret; - if (INTEL_INFO(dev)->gen < 5) - return -ENODEV; - intel_plane = kzalloc(sizeof(*intel_plane), GFP_KERNEL); if (!intel_plane) { ret = -ENOMEM; -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] drm/i915: Initialize planes in a reasonable order 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala 2016-10-25 15:58 ` [PATCH 1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk ville.syrjala @ 2016-10-25 15:58 ` ville.syrjala 2016-10-25 15:58 ` [PATCH 3/4] drm/i915: Bail if plane/crtc init fails ville.syrjala ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: ville.syrjala @ 2016-10-25 15:58 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> The zpos magic sorting uses the object ID to solve conflicting zpos values. Let's initialize our planes in an order that makes the object IDs agree with the normal primary->sprites->cursor z order. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2a5a7c2868de..244a0f59d8f7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15272,7 +15272,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) struct intel_crtc_state *crtc_state = NULL; struct drm_plane *primary = NULL; struct drm_plane *cursor = NULL; - int ret; + int sprite, ret; intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL); if (intel_crtc == NULL) @@ -15299,6 +15299,13 @@ static void intel_crtc_init(struct drm_device *dev, int pipe) if (!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); + } + cursor = intel_cursor_plane_create(dev, pipe); if (!cursor) goto fail; @@ -16423,7 +16430,6 @@ void intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); struct i915_ggtt *ggtt = &dev_priv->ggtt; - int sprite, ret; enum pipe pipe; struct intel_crtc *crtc; @@ -16494,12 +16500,6 @@ void intel_modeset_init(struct drm_device *dev) for_each_pipe(dev_priv, pipe) { intel_crtc_init(dev, pipe); - 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); - } } intel_update_czclk(dev_priv); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] drm/i915: Bail if plane/crtc init fails 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala 2016-10-25 15:58 ` [PATCH 1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk ville.syrjala 2016-10-25 15:58 ` [PATCH 2/4] drm/i915: Initialize planes in a reasonable order ville.syrjala @ 2016-10-25 15:58 ` ville.syrjala 2016-10-27 7:03 ` Daniel Vetter 2016-11-04 20:48 ` Chris Wilson 2016-10-25 15:58 ` [PATCH 4/4] drm/i915: Reorganize sprite init ville.syrjala 2016-10-25 16:16 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk Patchwork 4 siblings, 2 replies; 12+ messages in thread From: ville.syrjala @ 2016-10-25 15:58 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> 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. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 101 ++++++++++++++++++++++------------- drivers/gpu/drm/i915/intel_drv.h | 3 +- drivers/gpu/drm/i915/intel_sprite.c | 8 +-- 5 files changed, 75 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index af3559d34328..0e393b5a988a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -592,7 +592,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 7a621c74254e..a9308aeb2f1f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3700,7 +3700,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 244a0f59d8f7..d5a49d12dc88 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14975,9 +14975,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)); } @@ -14991,11 +14988,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; @@ -15006,12 +15002,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; @@ -15092,13 +15093,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 @@ -15194,8 +15195,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; @@ -15203,12 +15204,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; @@ -15240,13 +15246,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, @@ -15265,22 +15271,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; @@ -15296,22 +15304,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; @@ -15343,13 +15359,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) @@ -16426,7 +16447,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; @@ -16450,7 +16471,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 @@ -16499,7 +16520,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); @@ -16547,6 +16574,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 7dda79df55d0..4fd7d74fd6dc 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1779,7 +1779,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); } -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Bail if plane/crtc init fails 2016-10-25 15:58 ` [PATCH 3/4] drm/i915: Bail if plane/crtc init fails ville.syrjala @ 2016-10-27 7:03 ` Daniel Vetter 2016-11-04 20:48 ` Chris Wilson 1 sibling, 0 replies; 12+ messages in thread From: Daniel Vetter @ 2016-10-27 7:03 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Tue, Oct 25, 2016 at 06:58:02PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Some bikeshedding in this patch, but I like it. For patches 1-3: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/i915/i915_drv.c | 4 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 101 ++++++++++++++++++++++------------- > drivers/gpu/drm/i915/intel_drv.h | 3 +- > drivers/gpu/drm/i915/intel_sprite.c | 8 +-- > 5 files changed, 75 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index af3559d34328..0e393b5a988a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -592,7 +592,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 7a621c74254e..a9308aeb2f1f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3700,7 +3700,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 244a0f59d8f7..d5a49d12dc88 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -14975,9 +14975,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)); > } > @@ -14991,11 +14988,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; > @@ -15006,12 +15002,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; > @@ -15092,13 +15093,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 > @@ -15194,8 +15195,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; > @@ -15203,12 +15204,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; > @@ -15240,13 +15246,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, > @@ -15265,22 +15271,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; > @@ -15296,22 +15304,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; > @@ -15343,13 +15359,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) > @@ -16426,7 +16447,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; > @@ -16450,7 +16471,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 > @@ -16499,7 +16520,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); > @@ -16547,6 +16574,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 7dda79df55d0..4fd7d74fd6dc 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1779,7 +1779,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); > } > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Bail if plane/crtc init fails 2016-10-25 15:58 ` [PATCH 3/4] drm/i915: Bail if plane/crtc init fails ville.syrjala 2016-10-27 7:03 ` Daniel Vetter @ 2016-11-04 20:48 ` Chris Wilson 2016-11-04 21:07 ` Ville Syrjälä 1 sibling, 1 reply; 12+ messages in thread From: Chris Wilson @ 2016-11-04 20:48 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Tue, Oct 25, 2016 at 06:58:02PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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. smatch spotted ERR_PTR(0) > @@ -15296,22 +15304,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); Here... > 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); and here. -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Bail if plane/crtc init fails 2016-11-04 20:48 ` Chris Wilson @ 2016-11-04 21:07 ` Ville Syrjälä 2016-11-04 21:45 ` Chris Wilson 0 siblings, 1 reply; 12+ messages in thread From: Ville Syrjälä @ 2016-11-04 21:07 UTC (permalink / raw) To: Chris Wilson, intel-gfx On Fri, Nov 04, 2016 at 08:48:21PM +0000, Chris Wilson wrote: > On Tue, Oct 25, 2016 at 06:58:02PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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. > > smatch spotted ERR_PTR(0) > > > @@ -15296,22 +15304,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); > > Here... This looks correct to me, but the cursor and sprite paths are clearly crap. > > > 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); > > and here. > > -- > Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] drm/i915: Bail if plane/crtc init fails 2016-11-04 21:07 ` Ville Syrjälä @ 2016-11-04 21:45 ` Chris Wilson 0 siblings, 0 replies; 12+ messages in thread From: Chris Wilson @ 2016-11-04 21:45 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Fri, Nov 04, 2016 at 11:07:26PM +0200, Ville Syrjälä wrote: > On Fri, Nov 04, 2016 at 08:48:21PM +0000, Chris Wilson wrote: > > On Tue, Oct 25, 2016 at 06:58:02PM +0300, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > 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. > > > > smatch spotted ERR_PTR(0) > > > > > @@ -15296,22 +15304,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); > > > > Here... > > This looks correct to me, but the cursor and sprite paths are clearly > crap. Brain had already turned off. Yes, it was the plane and cursor, I just goofed in trimming. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] drm/i915: Reorganize sprite init 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala ` (2 preceding siblings ...) 2016-10-25 15:58 ` [PATCH 3/4] drm/i915: Bail if plane/crtc init fails ville.syrjala @ 2016-10-25 15:58 ` ville.syrjala 2016-10-27 7:07 ` Daniel Vetter 2016-10-25 16:16 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk Patchwork 4 siblings, 1 reply; 12+ messages in thread From: ville.syrjala @ 2016-10-25 15:58 UTC (permalink / raw) To: intel-gfx From: Ville Syrjälä <ville.syrjala@linux.intel.com> Kill the switch statement from the sprite init code and replace with a more straightforward if ladder. Now each significant evolution of the sprite hardware is in its own neat box. Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_sprite.c | 70 ++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 41ae7f562eec..70b50a27763e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1067,25 +1067,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) } intel_plane->base.state = &state->base; - switch (INTEL_INFO(dev)->gen) { - case 5: - case 6: + if (INTEL_GEN(dev_priv) >= 9) { intel_plane->can_scale = true; - intel_plane->max_downscale = 16; - intel_plane->update_plane = ilk_update_plane; - intel_plane->disable_plane = ilk_disable_plane; + state->scaler_id = -1; - if (IS_GEN6(dev_priv)) { - plane_formats = snb_plane_formats; - num_plane_formats = ARRAY_SIZE(snb_plane_formats); - } else { - plane_formats = ilk_plane_formats; - num_plane_formats = ARRAY_SIZE(ilk_plane_formats); - } - break; + intel_plane->update_plane = skl_update_plane; + intel_plane->disable_plane = skl_disable_plane; + + plane_formats = skl_plane_formats; + num_plane_formats = ARRAY_SIZE(skl_plane_formats); + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { + intel_plane->can_scale = false; + intel_plane->max_downscale = 1; + + intel_plane->update_plane = vlv_update_plane; + intel_plane->disable_plane = vlv_disable_plane; - case 7: - case 8: + plane_formats = vlv_plane_formats; + num_plane_formats = ARRAY_SIZE(vlv_plane_formats); + } else if (INTEL_GEN(dev_priv) >= 7) { if (IS_IVYBRIDGE(dev_priv)) { intel_plane->can_scale = true; intel_plane->max_downscale = 2; @@ -1094,33 +1094,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) intel_plane->max_downscale = 1; } - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { - intel_plane->update_plane = vlv_update_plane; - intel_plane->disable_plane = vlv_disable_plane; + intel_plane->update_plane = ivb_update_plane; + intel_plane->disable_plane = ivb_disable_plane; - plane_formats = vlv_plane_formats; - num_plane_formats = ARRAY_SIZE(vlv_plane_formats); - } else { - intel_plane->update_plane = ivb_update_plane; - intel_plane->disable_plane = ivb_disable_plane; + plane_formats = snb_plane_formats; + num_plane_formats = ARRAY_SIZE(snb_plane_formats); + } else { + intel_plane->can_scale = true; + intel_plane->max_downscale = 16; + + intel_plane->update_plane = ilk_update_plane; + intel_plane->disable_plane = ilk_disable_plane; + if (IS_GEN6(dev_priv)) { plane_formats = snb_plane_formats; num_plane_formats = ARRAY_SIZE(snb_plane_formats); + } else { + plane_formats = ilk_plane_formats; + num_plane_formats = ARRAY_SIZE(ilk_plane_formats); } - break; - case 9: - intel_plane->can_scale = true; - intel_plane->update_plane = skl_update_plane; - intel_plane->disable_plane = skl_disable_plane; - state->scaler_id = -1; - - plane_formats = skl_plane_formats; - num_plane_formats = ARRAY_SIZE(skl_plane_formats); - break; - default: - MISSING_CASE(INTEL_INFO(dev)->gen); - ret = -ENODEV; - goto fail; } if (INTEL_GEN(dev_priv) >= 9) { @@ -1139,7 +1131,7 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) possible_crtcs = (1 << pipe); - if (INTEL_INFO(dev)->gen >= 9) + if (INTEL_GEN(dev_priv) >= 9) ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, &intel_plane_funcs, plane_formats, num_plane_formats, -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/i915: Reorganize sprite init 2016-10-25 15:58 ` [PATCH 4/4] drm/i915: Reorganize sprite init ville.syrjala @ 2016-10-27 7:07 ` Daniel Vetter 2016-10-31 15:00 ` Ville Syrjälä 0 siblings, 1 reply; 12+ messages in thread From: Daniel Vetter @ 2016-10-27 7:07 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx On Tue, Oct 25, 2016 at 06:58:03PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Kill the switch statement from the sprite init code and replace with a > more straightforward if ladder. Now each significant evolution of the > sprite hardware is in its own neat box. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_sprite.c | 70 ++++++++++++++++--------------------- > 1 file changed, 31 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 41ae7f562eec..70b50a27763e 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -1067,25 +1067,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > } > intel_plane->base.state = &state->base; > > - switch (INTEL_INFO(dev)->gen) { > - case 5: > - case 6: > + if (INTEL_GEN(dev_priv) >= 9) { Maybe do an s/dev/dev_priv/ on the function paramaters while at it? With or wihtout that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > intel_plane->can_scale = true; > - intel_plane->max_downscale = 16; > - intel_plane->update_plane = ilk_update_plane; > - intel_plane->disable_plane = ilk_disable_plane; > + state->scaler_id = -1; > > - if (IS_GEN6(dev_priv)) { > - plane_formats = snb_plane_formats; > - num_plane_formats = ARRAY_SIZE(snb_plane_formats); > - } else { > - plane_formats = ilk_plane_formats; > - num_plane_formats = ARRAY_SIZE(ilk_plane_formats); > - } > - break; > + intel_plane->update_plane = skl_update_plane; > + intel_plane->disable_plane = skl_disable_plane; > + > + plane_formats = skl_plane_formats; > + num_plane_formats = ARRAY_SIZE(skl_plane_formats); > + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > + intel_plane->can_scale = false; > + intel_plane->max_downscale = 1; > + > + intel_plane->update_plane = vlv_update_plane; > + intel_plane->disable_plane = vlv_disable_plane; > > - case 7: > - case 8: > + plane_formats = vlv_plane_formats; > + num_plane_formats = ARRAY_SIZE(vlv_plane_formats); > + } else if (INTEL_GEN(dev_priv) >= 7) { > if (IS_IVYBRIDGE(dev_priv)) { > intel_plane->can_scale = true; > intel_plane->max_downscale = 2; > @@ -1094,33 +1094,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > intel_plane->max_downscale = 1; > } > > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > - intel_plane->update_plane = vlv_update_plane; > - intel_plane->disable_plane = vlv_disable_plane; > + intel_plane->update_plane = ivb_update_plane; > + intel_plane->disable_plane = ivb_disable_plane; > > - plane_formats = vlv_plane_formats; > - num_plane_formats = ARRAY_SIZE(vlv_plane_formats); > - } else { > - intel_plane->update_plane = ivb_update_plane; > - intel_plane->disable_plane = ivb_disable_plane; > + plane_formats = snb_plane_formats; > + num_plane_formats = ARRAY_SIZE(snb_plane_formats); > + } else { > + intel_plane->can_scale = true; > + intel_plane->max_downscale = 16; > + > + intel_plane->update_plane = ilk_update_plane; > + intel_plane->disable_plane = ilk_disable_plane; > > + if (IS_GEN6(dev_priv)) { > plane_formats = snb_plane_formats; > num_plane_formats = ARRAY_SIZE(snb_plane_formats); > + } else { > + plane_formats = ilk_plane_formats; > + num_plane_formats = ARRAY_SIZE(ilk_plane_formats); > } > - break; > - case 9: > - intel_plane->can_scale = true; > - intel_plane->update_plane = skl_update_plane; > - intel_plane->disable_plane = skl_disable_plane; > - state->scaler_id = -1; > - > - plane_formats = skl_plane_formats; > - num_plane_formats = ARRAY_SIZE(skl_plane_formats); > - break; > - default: > - MISSING_CASE(INTEL_INFO(dev)->gen); > - ret = -ENODEV; > - goto fail; > } > > if (INTEL_GEN(dev_priv) >= 9) { > @@ -1139,7 +1131,7 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > > possible_crtcs = (1 << pipe); > > - if (INTEL_INFO(dev)->gen >= 9) > + if (INTEL_GEN(dev_priv) >= 9) > ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, > &intel_plane_funcs, > plane_formats, num_plane_formats, > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] drm/i915: Reorganize sprite init 2016-10-27 7:07 ` Daniel Vetter @ 2016-10-31 15:00 ` Ville Syrjälä 0 siblings, 0 replies; 12+ messages in thread From: Ville Syrjälä @ 2016-10-31 15:00 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Thu, Oct 27, 2016 at 09:07:13AM +0200, Daniel Vetter wrote: > On Tue, Oct 25, 2016 at 06:58:03PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Kill the switch statement from the sprite init code and replace with a > > more straightforward if ladder. Now each significant evolution of the > > sprite hardware is in its own neat box. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_sprite.c | 70 ++++++++++++++++--------------------- > > 1 file changed, 31 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > > index 41ae7f562eec..70b50a27763e 100644 > > --- a/drivers/gpu/drm/i915/intel_sprite.c > > +++ b/drivers/gpu/drm/i915/intel_sprite.c > > @@ -1067,25 +1067,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > > } > > intel_plane->base.state = &state->base; > > > > - switch (INTEL_INFO(dev)->gen) { > > - case 5: > > - case 6: > > + if (INTEL_GEN(dev_priv) >= 9) { > > Maybe do an s/dev/dev_priv/ on the function paramaters while at it? With > or wihtout that: I went without. Looks like the change would like to propagate quite a bit further, so I figured I'd so a separate series for it. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Series pushed to dinq. Thanls for the review. > > > > intel_plane->can_scale = true; > > - intel_plane->max_downscale = 16; > > - intel_plane->update_plane = ilk_update_plane; > > - intel_plane->disable_plane = ilk_disable_plane; > > + state->scaler_id = -1; > > > > - if (IS_GEN6(dev_priv)) { > > - plane_formats = snb_plane_formats; > > - num_plane_formats = ARRAY_SIZE(snb_plane_formats); > > - } else { > > - plane_formats = ilk_plane_formats; > > - num_plane_formats = ARRAY_SIZE(ilk_plane_formats); > > - } > > - break; > > + intel_plane->update_plane = skl_update_plane; > > + intel_plane->disable_plane = skl_disable_plane; > > + > > + plane_formats = skl_plane_formats; > > + num_plane_formats = ARRAY_SIZE(skl_plane_formats); > > + } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > + intel_plane->can_scale = false; > > + intel_plane->max_downscale = 1; > > + > > + intel_plane->update_plane = vlv_update_plane; > > + intel_plane->disable_plane = vlv_disable_plane; > > > > - case 7: > > - case 8: > > + plane_formats = vlv_plane_formats; > > + num_plane_formats = ARRAY_SIZE(vlv_plane_formats); > > + } else if (INTEL_GEN(dev_priv) >= 7) { > > if (IS_IVYBRIDGE(dev_priv)) { > > intel_plane->can_scale = true; > > intel_plane->max_downscale = 2; > > @@ -1094,33 +1094,25 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > > intel_plane->max_downscale = 1; > > } > > > > - if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > > - intel_plane->update_plane = vlv_update_plane; > > - intel_plane->disable_plane = vlv_disable_plane; > > + intel_plane->update_plane = ivb_update_plane; > > + intel_plane->disable_plane = ivb_disable_plane; > > > > - plane_formats = vlv_plane_formats; > > - num_plane_formats = ARRAY_SIZE(vlv_plane_formats); > > - } else { > > - intel_plane->update_plane = ivb_update_plane; > > - intel_plane->disable_plane = ivb_disable_plane; > > + plane_formats = snb_plane_formats; > > + num_plane_formats = ARRAY_SIZE(snb_plane_formats); > > + } else { > > + intel_plane->can_scale = true; > > + intel_plane->max_downscale = 16; > > + > > + intel_plane->update_plane = ilk_update_plane; > > + intel_plane->disable_plane = ilk_disable_plane; > > > > + if (IS_GEN6(dev_priv)) { > > plane_formats = snb_plane_formats; > > num_plane_formats = ARRAY_SIZE(snb_plane_formats); > > + } else { > > + plane_formats = ilk_plane_formats; > > + num_plane_formats = ARRAY_SIZE(ilk_plane_formats); > > } > > - break; > > - case 9: > > - intel_plane->can_scale = true; > > - intel_plane->update_plane = skl_update_plane; > > - intel_plane->disable_plane = skl_disable_plane; > > - state->scaler_id = -1; > > - > > - plane_formats = skl_plane_formats; > > - num_plane_formats = ARRAY_SIZE(skl_plane_formats); > > - break; > > - default: > > - MISSING_CASE(INTEL_INFO(dev)->gen); > > - ret = -ENODEV; > > - goto fail; > > } > > > > if (INTEL_GEN(dev_priv) >= 9) { > > @@ -1139,7 +1131,7 @@ intel_sprite_plane_create(struct drm_device *dev, enum pipe pipe, int plane) > > > > possible_crtcs = (1 << pipe); > > > > - if (INTEL_INFO(dev)->gen >= 9) > > + if (INTEL_GEN(dev_priv) >= 9) > > ret = drm_universal_plane_init(dev, &intel_plane->base, possible_crtcs, > > &intel_plane_funcs, > > plane_formats, num_plane_formats, > > -- > > 2.7.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala ` (3 preceding siblings ...) 2016-10-25 15:58 ` [PATCH 4/4] drm/i915: Reorganize sprite init ville.syrjala @ 2016-10-25 16:16 ` Patchwork 4 siblings, 0 replies; 12+ messages in thread From: Patchwork @ 2016-10-25 16:16 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx == Series Details == Series: series starting with [1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk URL : https://patchwork.freedesktop.org/series/14347/ State : warning == Summary == Series 14347v1 Series without cover letter https://patchwork.freedesktop.org/api/1.0/series/14347/revisions/1/mbox/ Test drv_module_reload_basic: dmesg-warn -> PASS (fi-skl-6770hq) Test gem_exec_suspend: Subgroup basic-s3: dmesg-warn -> PASS (fi-skl-6770hq) Test kms_flip: Subgroup basic-flip-vs-wf_vblank: pass -> SKIP (fi-byt-n2820) Test kms_frontbuffer_tracking: Subgroup basic: fail -> PASS (fi-skl-6770hq) fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42 fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30 fi-byt-j1900 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31 fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36 fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22 fi-hsw-4770r total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23 fi-ilk-650 total:246 pass:185 dwarn:0 dfail:0 fail:0 skip:61 fi-ivb-3520m total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26 fi-ivb-3770 total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26 fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24 fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:246 pass:219 dwarn:4 dfail:0 fail:0 skip:23 fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23 fi-skl-6770hq total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37 fi-snb-2600 total:246 pass:208 dwarn:0 dfail:0 fail:0 skip:38 3ce99d02d8f2b7d1414d20d5972f8e277b33693e drm-intel-nightly: 2016y-10m-25d-13h-55m-46s UTC integration manifest 1d8506a drm/i915: Reorganize sprite init 28f60dd drm/i915: Bail if plane/crtc init fails df93569 drm/i915: Initialize planes in a reasonable order 59d4786 drm/i915: Don't try to initialize sprite planes on pre-ilk Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2813/ == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2813/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-11-04 21:45 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-25 15:57 [PATCH 0/4] drm/i915: Abort if crtc/plane init fails ville.syrjala 2016-10-25 15:58 ` [PATCH 1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk ville.syrjala 2016-10-25 15:58 ` [PATCH 2/4] drm/i915: Initialize planes in a reasonable order ville.syrjala 2016-10-25 15:58 ` [PATCH 3/4] drm/i915: Bail if plane/crtc init fails ville.syrjala 2016-10-27 7:03 ` Daniel Vetter 2016-11-04 20:48 ` Chris Wilson 2016-11-04 21:07 ` Ville Syrjälä 2016-11-04 21:45 ` Chris Wilson 2016-10-25 15:58 ` [PATCH 4/4] drm/i915: Reorganize sprite init ville.syrjala 2016-10-27 7:07 ` Daniel Vetter 2016-10-31 15:00 ` Ville Syrjälä 2016-10-25 16:16 ` ✗ Fi.CI.BAT: warning for series starting with [1/4] drm/i915: Don't try to initialize sprite planes on pre-ilk Patchwork
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.