I sometimes see:
[drm:drm_framebuffer_remove [drm]] *ERROR* failed to reset crtc ed2a6c00 when fb was deleted: -22
which comes from drm_framebuffer_remove() when it's disabling the crtc with zeroed drm_mode_set.
The problem in omap_plane_atomic_check() is that it will use those zeroed fields to verify if the setup is correct.
This patch makes omap_plane_atomic_check() return 0 if the crtc is disabled.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/omap_plane.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 5252ab720e70..85eab82f91bc 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -168,6 +168,9 @@ static int omap_plane_atomic_check(struct drm_plane *plane, if (IS_ERR(crtc_state)) return PTR_ERR(crtc_state);
+ if (!crtc_state->enable) + return 0; + if (state->crtc_x < 0 || state->crtc_y < 0) return -EINVAL;
Clean up omap_plane_atomic_check() with:
- Check state->fb first. If no fb, return 0. - use drm_atomic_get_existing_crtc_state() instead of drm_atomic_get_crtc_state()
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/omap_plane.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 85eab82f91bc..5664200ef05b 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -161,12 +161,17 @@ static int omap_plane_atomic_check(struct drm_plane *plane, { struct drm_crtc_state *crtc_state;
- if (!state->crtc) + if (!state->fb) return 0;
- crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); - if (IS_ERR(crtc_state)) - return PTR_ERR(crtc_state); + /* crtc should only be NULL when disabling (i.e., !state->fb) */ + if (WARN_ON(!state->crtc)) + return 0; + + crtc_state = drm_atomic_get_existing_crtc_state(state->state, state->crtc); + /* we should have a crtc state if the plane is attached to a crtc */ + if (WARN_ON(!crtc_state)) + return 0;
if (!crtc_state->enable) return 0; @@ -180,11 +185,9 @@ static int omap_plane_atomic_check(struct drm_plane *plane, if (state->crtc_y + state->crtc_h > crtc_state->adjusted_mode.vdisplay) return -EINVAL;
- if (state->fb) { - if (state->rotation != BIT(DRM_ROTATE_0) && - !omap_framebuffer_supports_rotation(state->fb)) - return -EINVAL; - } + if (state->rotation != BIT(DRM_ROTATE_0) && + !omap_framebuffer_supports_rotation(state->fb)) + return -EINVAL;
return 0; }
omap_plane_atomic_update() does WARN_ON() if dispc rejects the given plane config. Change that to dev_err() to lessen the possible spam.
To fix this correctly, the plane setup needs much more work by creating a check function for dispc setup, so that we could reliably check the config in atomic_check, instead of only noticing the problem when programming dispc.
Signed-off-by: Tomi Valkeinen tomi.valkeinen@ti.com --- drivers/gpu/drm/omapdrm/omap_plane.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c index 5664200ef05b..4f39855d80fe 100644 --- a/drivers/gpu/drm/omapdrm/omap_plane.c +++ b/drivers/gpu/drm/omapdrm/omap_plane.c @@ -135,7 +135,9 @@ static void omap_plane_atomic_update(struct drm_plane *plane, /* and finally, update omapdss: */ ret = dispc_ovl_setup(omap_plane->id, &info, false, omap_crtc_timings(state->crtc), false); - if (WARN_ON(ret)) { + if (ret) { + dev_err(plane->dev->dev, "Failed to setup plane %s\n", + omap_plane->name); dispc_ovl_enable(omap_plane->id, false); return; }
dri-devel@lists.freedesktop.org
-
Tomi Valkeinen