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@tronnes.org
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.