Den 09.05.2016 16:46, skrev Daniel Vetter: > On Thu, May 05, 2016 at 03:24:33PM +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 at tronnes.org> >> +static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, >> + struct drm_plane_state *pstate) >> +{ >> + struct drm_simple_display_pipe *pipe; >> + struct drm_crtc_state *cstate; >> + >> + pipe = container_of(plane, struct drm_simple_display_pipe, plane); >> + if (!pipe->funcs || !pipe->funcs->check) >> + return 0; >> + >> + cstate = drm_atomic_get_existing_crtc_state(pstate->state, >> + &pipe->crtc); >> + >> + return pipe->funcs->check(pipe, pstate, cstate); >> +} > Ok one thing I've missed here is that for most drivers this is way too > simple a check function, which means we'll end up with tons of duplicated > code. Things which the drm core allows, but simple pipelines all don't > really cope with: > - plane scaling > - disabling the plane without the crtc (i.e. scan out black) > - plane not sized to fill the entire hactive/vactive > > There's a helper to do most of these checks for you - > drm_plane_helper_check_update. I think it'd be good to place a call for > that in here, before we call down into the driver's ->check callback. But > ofc before we return 0; we want these checks always done. And catch all > these things so that drivers never fall over this pitfall.
Does this resemble what you're after? I'm just guessing here. static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *pstate) { struct drm_rect src = { .x1 = pstate->src_x, .y1 = pstate->src_y, .x2 = pstate->src_x + pstate->src_w, .y2 = pstate->src_y + pstate->src_h, }; struct drm_rect dest = { .x1 = pstate->crtc_x, .y1 = pstate->crtc_y, .x2 = pstate->crtc_x + pstate->crtc_w, .y2 = pstate->crtc_y + pstate->crtc_h, }; struct drm_rect clip = { 0 }; struct drm_simple_display_pipe *pipe; struct drm_crtc_state *cstate; bool visible; int ret; pipe = container_of(plane, struct drm_simple_display_pipe, plane); clip.x2 = pipe->crtc.mode.hdisplay; clip.y2 = pipe->crtc.mode.vdisplay; ret = drm_plane_helper_check_update(plane, &pipe->crtc, plane->fb, &src, &dest, &clip, DRM_PLANE_HELPER_NO_SCALING, DRM_PLANE_HELPER_NO_SCALING, false, false, &visible); if (ret) return ret; /* How to handle !visible, is it even possible? */ if (!pipe->funcs || !pipe->funcs->check) return 0; cstate = drm_atomic_get_existing_crtc_state(pstate->state, &pipe->crtc); return pipe->funcs->check(pipe, pstate, cstate); } Noralf. > Noticed while discussing tilcdc atomic patches, since tilcdc could > probably use drm_simple_display_pipe too. > -Daniel