On Wed, Dec 20, 2017 at 10:35:45AM +0100, Maarten Lankhorst wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > We don't need any active planes during load detection, so just disable > them all. This saves us from having to come up with a suitable > framebuffer. And we also avoid leaving sprite/cursor planes on and > potentially presenting them at a peculiar location during the load > detection. > > Changes since v1 (Maarten): > - Add missing call to add_all_affected_planes. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102707 Less code, I like. And I think we have enough load-detect machines (+ plus the nasty igt to do it anywhere we still have native vga) to have reasonable ensurance it actually works. But maybe let's soak this in -next first for a while, then cherry-pick over after a few weeks once it's solid. With the missing Fixes: line added. Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 147 +++++------------------------------ > 1 file changed, 18 insertions(+), 129 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7e833268c9df..ef61f9a75c93 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9689,111 +9689,27 @@ intel_framebuffer_create(struct drm_i915_gem_object *obj, > return ERR_PTR(ret); > } > > -static u32 > -intel_framebuffer_pitch_for_width(int width, int bpp) > -{ > - u32 pitch = DIV_ROUND_UP(width * bpp, 8); > - return ALIGN(pitch, 64); > -} > - > -static u32 > -intel_framebuffer_size_for_mode(const struct drm_display_mode *mode, int bpp) > -{ > - u32 pitch = intel_framebuffer_pitch_for_width(mode->hdisplay, bpp); > - return PAGE_ALIGN(pitch * mode->vdisplay); > -} > - > -static struct drm_framebuffer * > -intel_framebuffer_create_for_mode(struct drm_device *dev, > - const struct drm_display_mode *mode, > - int depth, int bpp) > -{ > - struct drm_framebuffer *fb; > - struct drm_i915_gem_object *obj; > - struct drm_mode_fb_cmd2 mode_cmd = { 0 }; > - > - obj = i915_gem_object_create(to_i915(dev), > - intel_framebuffer_size_for_mode(mode, bpp)); > - if (IS_ERR(obj)) > - return ERR_CAST(obj); > - > - mode_cmd.width = mode->hdisplay; > - mode_cmd.height = mode->vdisplay; > - mode_cmd.pitches[0] = intel_framebuffer_pitch_for_width(mode_cmd.width, > - bpp); > - mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth); > - > - fb = intel_framebuffer_create(obj, &mode_cmd); > - if (IS_ERR(fb)) > - i915_gem_object_put(obj); > - > - return fb; > -} > - > -static struct drm_framebuffer * > -mode_fits_in_fbdev(struct drm_device *dev, > - const struct drm_display_mode *mode) > -{ > -#ifdef CONFIG_DRM_FBDEV_EMULATION > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct drm_i915_gem_object *obj; > - struct drm_framebuffer *fb; > - > - if (!dev_priv->fbdev) > - return NULL; > - > - if (!dev_priv->fbdev->fb) > - return NULL; > - > - obj = dev_priv->fbdev->fb->obj; > - BUG_ON(!obj); > - > - fb = &dev_priv->fbdev->fb->base; > - if (fb->pitches[0] < intel_framebuffer_pitch_for_width(mode->hdisplay, > - fb->format->cpp[0] * 8)) > - return NULL; > - > - if (obj->base.size < mode->vdisplay * fb->pitches[0]) > - return NULL; > - > - drm_framebuffer_get(fb); > - return fb; > -#else > - return NULL; > -#endif > -} > - > -static int intel_modeset_setup_plane_state(struct drm_atomic_state *state, > - struct drm_crtc *crtc, > - const struct drm_display_mode *mode, > - struct drm_framebuffer *fb, > - int x, int y) > +static int intel_modeset_disable_planes(struct drm_atomic_state *state, > + struct drm_crtc *crtc) > { > + struct drm_plane *plane; > struct drm_plane_state *plane_state; > - int hdisplay, vdisplay; > - int ret; > - > - plane_state = drm_atomic_get_plane_state(state, crtc->primary); > - if (IS_ERR(plane_state)) > - return PTR_ERR(plane_state); > - > - if (mode) > - drm_mode_get_hv_timing(mode, &hdisplay, &vdisplay); > - else > - hdisplay = vdisplay = 0; > + int ret, i; > > - ret = drm_atomic_set_crtc_for_plane(plane_state, fb ? crtc : NULL); > + ret = drm_atomic_add_affected_planes(state, crtc); > if (ret) > return ret; > - drm_atomic_set_fb_for_plane(plane_state, fb); > - plane_state->crtc_x = 0; > - plane_state->crtc_y = 0; > - plane_state->crtc_w = hdisplay; > - plane_state->crtc_h = vdisplay; > - plane_state->src_x = x << 16; > - plane_state->src_y = y << 16; > - plane_state->src_w = hdisplay << 16; > - plane_state->src_h = vdisplay << 16; > + > + for_each_new_plane_in_state(state, plane, plane_state, i) { > + if (plane_state->crtc != crtc) > + continue; > + > + ret = drm_atomic_set_crtc_for_plane(plane_state, NULL); > + if (ret) > + return ret; > + > + drm_atomic_set_fb_for_plane(plane_state, NULL); > + } > > return 0; > } > @@ -9811,7 +9727,6 @@ int intel_get_load_detect_pipe(struct drm_connector *connector, > struct drm_crtc *crtc = NULL; > struct drm_device *dev = encoder->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > - struct drm_framebuffer *fb; > struct drm_mode_config *config = &dev->mode_config; > struct drm_atomic_state *state = NULL, *restore_state = NULL; > struct drm_connector_state *connector_state; > @@ -9879,10 +9794,6 @@ int intel_get_load_detect_pipe(struct drm_connector *connector, > found: > intel_crtc = to_intel_crtc(crtc); > > - ret = drm_modeset_lock(&crtc->primary->mutex, ctx); > - if (ret) > - goto fail; > - > state = drm_atomic_state_alloc(dev); > restore_state = drm_atomic_state_alloc(dev); > if (!state || !restore_state) { > @@ -9914,39 +9825,17 @@ int intel_get_load_detect_pipe(struct drm_connector *connector, > if (!mode) > mode = &load_detect_mode; > > - /* We need a framebuffer large enough to accommodate all accesses > - * that the plane may generate whilst we perform load detection. > - * We can not rely on the fbcon either being present (we get called > - * during its initialisation to detect all boot displays, or it may > - * not even exist) or that it is large enough to satisfy the > - * requested mode. > - */ > - fb = mode_fits_in_fbdev(dev, mode); > - if (fb == NULL) { > - DRM_DEBUG_KMS("creating tmp fb for load-detection\n"); > - fb = intel_framebuffer_create_for_mode(dev, mode, 24, 32); > - } else > - DRM_DEBUG_KMS("reusing fbdev for load-detection framebuffer\n"); > - if (IS_ERR(fb)) { > - DRM_DEBUG_KMS("failed to allocate framebuffer for load-detection\n"); > - ret = PTR_ERR(fb); > - goto fail; > - } > - > - ret = intel_modeset_setup_plane_state(state, crtc, mode, fb, 0, 0); > - drm_framebuffer_put(fb); > + ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); > if (ret) > goto fail; > > - ret = drm_atomic_set_mode_for_crtc(&crtc_state->base, mode); > + ret = intel_modeset_disable_planes(state, crtc); > if (ret) > goto fail; > > ret = PTR_ERR_OR_ZERO(drm_atomic_get_connector_state(restore_state, connector)); > if (!ret) > ret = PTR_ERR_OR_ZERO(drm_atomic_get_crtc_state(restore_state, crtc)); > - if (!ret) > - ret = PTR_ERR_OR_ZERO(drm_atomic_get_plane_state(restore_state, crtc->primary)); > if (ret) { > DRM_DEBUG_KMS("Failed to create a copy of old state to restore: %i\n", ret); > goto fail; > -- > 2.15.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel