Den 11.05.2016 19:09, skrev Daniel Vetter:
On Wed, May 11, 2016 at 06:09:22PM +0200, Noralf Trønnes wrote:
Provides helper functions for drivers that have a simple display
pipeline. Plane, crtc and encoder are collapsed into one entity.
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
Looks really nice, just a few suggestions for the kerneldoc. Would be
awesome if we could get an ack on this from Jyri for tilcdc, but even
without that I think I'll just pull in the next iteration. Still please cc
him.
Thanks, Daniel
Thanks for helping me with the docs.
[...]
+static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
+ struct drm_plane_state *plane_state)
+{
+ struct drm_rect src = {
+ .x1 = plane_state->src_x,
+ .y1 = plane_state->src_y,
+ .x2 = plane_state->src_x + plane_state->src_w,
+ .y2 = plane_state->src_y + plane_state->src_h,
+ };
+ struct drm_rect dest = {
+ .x1 = plane_state->crtc_x,
+ .y1 = plane_state->crtc_y,
+ .x2 = plane_state->crtc_x + plane_state->crtc_w,
+ .y2 = plane_state->crtc_y + plane_state->crtc_h,
+ };
+ struct drm_rect clip = { 0 };
+ struct drm_simple_display_pipe *pipe;
+ struct drm_crtc_state *crtc_state;
+ bool visible;
+ int ret;
+
+ pipe = container_of(plane, struct drm_simple_display_pipe, plane);
+ crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
+ &pipe->crtc);
+ if (crtc_state->enable != !!plane_state->crtc)
+ return -EINVAL; /* plane must match crtc enable state */
+
+ if (!crtc_state->enable)
+ return 0; /* nothing to check when disabling or disabled */
+
+ clip.x2 = crtc_state->adjusted_mode.hdisplay;
+ clip.y2 = crtc_state->adjusted_mode.vdisplay;
+ ret = drm_plane_helper_check_update(plane, &pipe->crtc,
+ plane_state->fb,
+ &src, &dest, &clip,
+ DRM_PLANE_HELPER_NO_SCALING,
+ DRM_PLANE_HELPER_NO_SCALING,
+ false, true, &visible);
+ if (ret)
+ return ret;
+
+ if (!visible)
+ return -EINVAL;
I think the logic above looks correct now, but might be worth checking
with your driver that it doesn't let something silly through. I.e. a small
test app that tries to reposition the primary plane, or tries to disable
it while the crtc is on. We should have that somewhere in libdrm I think.
I hacked libdrm tests/kms/kms-universal-planes to position the plane
at (1,1) which failed (I added some debug output to the driver):
[ 885.906697] [drm:drm_atomic_set_fb_for_plane] Set [FB:25] for plane
state b84aec40
[ 885.906707] [drm:drm_atomic_check_only] checking b84aeec0
[ 885.906738] [drm:drm_plane_helper_check_update] Plane must cover
entire CRTC
[ 885.906748] [drm:drm_rect_debug_print] dst: 319x239+1+1
[ 885.906757] [drm:drm_rect_debug_print] clip: 320x240+0+0
[ 885.906765] drm_simple_kms_plane_atomic_check: ret=-22, visible=1
[ 885.906775] [drm:drm_atomic_helper_check_planes] [PLANE:19:plane-0]
atomic driver check failed
Then I changed the test to pass 0 for fb id which also failed:
[ 3144.599790] [drm:drm_atomic_set_fb_for_plane] Set [NOFB] for plane
state b87805c0
[ 3144.599799] [drm:drm_atomic_check_only] checking b8780d80
[ 3144.599816] [drm:drm_atomic_helper_check_planes] [PLANE:19:plane-0]
atomic driver check failed
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
