On Thu, Feb 05, 2015 at 05:22:19PM +0000, Damien Lespiau wrote:
> Right now, we get a warning when taking over the firmware fb:
>
> [drm:drm_atomic_plane_check] FB set but no CRTC
>
> with the following backtrace:
>
> [<ffffffffa010339d>] drm_atomic_check_only+0x35d/0x510 [drm]
> [<ffffffffa0103567>] drm_atomic_commit+0x17/0x60 [drm]
> [<ffffffffa00a6ccd>] drm_atomic_helper_plane_set_property+0x8d/0xd0
> [drm_kms_helper]
> [<ffffffffa00f1fed>] drm_mode_plane_set_obj_prop+0x2d/0x90 [drm]
> [<ffffffffa00a8a1b>] restore_fbdev_mode+0x6b/0xf0 [drm_kms_helper]
> [<ffffffffa00aa969>] drm_fb_helper_restore_fbdev_mode_unlocked+0x29/0x80
> [drm_kms_helper]
> [<ffffffffa00aa9e2>] drm_fb_helper_set_par+0x22/0x50 [drm_kms_helper]
> [<ffffffffa050a71a>] intel_fbdev_set_par+0x1a/0x60 [i915]
> [<ffffffff813ad444>] fbcon_init+0x4f4/0x580
>
> That's because we update the plane state with the fb from the firmware, but we
> never associate the plane to that CRTC.
>
> We don't quite have the full DRM take over from HW state just yet, so
> fake enough of the plane atomic state to pass the checks.
>
> Signed-off-by: Damien Lespiau <damien.lesp...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 0d07535..eed1c0c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2439,8 +2439,12 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
> return;
>
> if (intel_alloc_plane_obj(intel_crtc, plane_config)) {
> - intel_crtc->base.primary->fb = &plane_config->fb->base;
> - update_state_fb(intel_crtc->base.primary);
> + struct drm_plane *primary = intel_crtc->base.primary;
> +
> + primary->fb = &plane_config->fb->base;
> + primary->state->crtc = &intel_crtc->base;
Would
drm_atomic_set_crtc_for_plane(primary->state, &intel_crtc->base)
be better here so that the various bitmasks get updated?
> + update_state_fb(primary);
> +
> return;
> }
>
> @@ -2469,6 +2473,7 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc,
>
> drm_framebuffer_reference(c->primary->fb);
> intel_crtc->base.primary->fb = c->primary->fb;
> + c->state->crtc = &intel_crtc->base;
I'm not sure I understand this line. c->state is the crtc state for the
other CRTC we're going to share with with (and has a backpointer of
c->state->crtc = c). Was this supposed to be something like
drm_atomic_set_crtc_for_plane(intel_crtc->base.primary->state,
&intel_crtc->base)
instead? I'm not sure if updating the other crtc (that we're sharing
with) was a mistake here.
Matt
> update_state_fb(intel_crtc->base.primary);
> obj->frontbuffer_bits |=
> INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
> break;
> --
> 1.8.3.1
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
