Commit d9be05b7 authored by Ville Syrjälä's avatar Ville Syrjälä
Browse files

drm/atomic: Use explicit old/new state in drm_atomic_plane_check() 



Convert drm_atomic_plane_check() over to using explicit old vs. new
plane states. Avoids the confusion of "what does plane->state mean
again?".

v2: Stick to the multi-stage logic in plane_switching_crtc() (Daniel)

Signed-off-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20181106191624.2207-1-ville.syrjala@linux.intel.com
parent b2432adf

drivers/gpu/drm/drm_atomic.c

+46 −39
Original line number Diff line number Diff line
@@ -487,14 +487,13 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state, 
EXPORT_SYMBOL(drm_atomic_get_plane_state);



static bool

plane_switching_crtc(struct drm_atomic_state *state,

		     struct drm_plane *plane,

		     struct drm_plane_state *plane_state)

plane_switching_crtc(const struct drm_plane_state *old_plane_state,

		     const struct drm_plane_state *new_plane_state)

{

	if (!plane->state->crtc || !plane_state->crtc)

	if (!old_plane_state->crtc || !new_plane_state->crtc)

		return false;



	if (plane->state->crtc == plane_state->crtc)

	if (old_plane_state->crtc == new_plane_state->crtc)

		return false;



	/* This could be refined, but currently there's no helper or driver code
@@ -507,88 +506,95 @@ plane_switching_crtc(struct drm_atomic_state *state, 


/**

 * drm_atomic_plane_check - check plane state

 * @plane: plane to check

 * @state: plane state to check

 * @old_plane_state: old plane state to check

 * @new_plane_state: new plane state to check

 *

 * Provides core sanity checks for plane state.

 *

 * RETURNS:

 * Zero on success, error code on failure

 */

static int drm_atomic_plane_check(struct drm_plane *plane,

		struct drm_plane_state *state)

static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,

				  const struct drm_plane_state *new_plane_state)

{

	struct drm_plane *plane = new_plane_state->plane;

	struct drm_crtc *crtc = new_plane_state->crtc;

	const struct drm_framebuffer *fb = new_plane_state->fb;

	unsigned int fb_width, fb_height;

	int ret;



	/* either *both* CRTC and FB must be set, or neither */

	if (state->crtc && !state->fb) {

	if (crtc && !fb) {

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] CRTC set but no FB\n",

				 plane->base.id, plane->name);

		return -EINVAL;

	} else if (state->fb && !state->crtc) {

	} else if (fb && !crtc) {

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] FB set but no CRTC\n",

				 plane->base.id, plane->name);

		return -EINVAL;

	}



	/* if disabled, we don't care about the rest of the state: */

	if (!state->crtc)

	if (!crtc)

		return 0;



	/* Check whether this plane is usable on this CRTC */

	if (!(plane->possible_crtcs & drm_crtc_mask(state->crtc))) {

	if (!(plane->possible_crtcs & drm_crtc_mask(crtc))) {

		DRM_DEBUG_ATOMIC("Invalid [CRTC:%d:%s] for [PLANE:%d:%s]\n",

				 state->crtc->base.id, state->crtc->name,

				 crtc->base.id, crtc->name,

				 plane->base.id, plane->name);

		return -EINVAL;

	}



	/* Check whether this plane supports the fb pixel format. */

	ret = drm_plane_check_pixel_format(plane, state->fb->format->format,

					   state->fb->modifier);

	ret = drm_plane_check_pixel_format(plane, fb->format->format,

					   fb->modifier);

	if (ret) {

		struct drm_format_name_buf format_name;

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] invalid pixel format %s, modifier 0x%llx\n",

				 plane->base.id, plane->name,

				 drm_get_format_name(state->fb->format->format,

				 drm_get_format_name(fb->format->format,

						     &format_name),

				 state->fb->modifier);

				 fb->modifier);

		return ret;

	}



	/* Give drivers some help against integer overflows */

	if (state->crtc_w > INT_MAX ||

	    state->crtc_x > INT_MAX - (int32_t) state->crtc_w ||

	    state->crtc_h > INT_MAX ||

	    state->crtc_y > INT_MAX - (int32_t) state->crtc_h) {

	if (new_plane_state->crtc_w > INT_MAX ||

	    new_plane_state->crtc_x > INT_MAX - (int32_t) new_plane_state->crtc_w ||

	    new_plane_state->crtc_h > INT_MAX ||

	    new_plane_state->crtc_y > INT_MAX - (int32_t) new_plane_state->crtc_h) {

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] invalid CRTC coordinates %ux%u+%d+%d\n",

				 plane->base.id, plane->name,

				 state->crtc_w, state->crtc_h,

				 state->crtc_x, state->crtc_y);

				 new_plane_state->crtc_w, new_plane_state->crtc_h,

				 new_plane_state->crtc_x, new_plane_state->crtc_y);

		return -ERANGE;

	}



	fb_width = state->fb->width << 16;

	fb_height = state->fb->height << 16;

	fb_width = fb->width << 16;

	fb_height = fb->height << 16;



	/* Make sure source coordinates are inside the fb. */

	if (state->src_w > fb_width ||

	    state->src_x > fb_width - state->src_w ||

	    state->src_h > fb_height ||

	    state->src_y > fb_height - state->src_h) {

	if (new_plane_state->src_w > fb_width ||

	    new_plane_state->src_x > fb_width - new_plane_state->src_w ||

	    new_plane_state->src_h > fb_height ||

	    new_plane_state->src_y > fb_height - new_plane_state->src_h) {

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] invalid source coordinates "

				 "%u.%06ux%u.%06u+%u.%06u+%u.%06u (fb %ux%u)\n",

				 plane->base.id, plane->name,

				 state->src_w >> 16, ((state->src_w & 0xffff) * 15625) >> 10,

				 state->src_h >> 16, ((state->src_h & 0xffff) * 15625) >> 10,

				 state->src_x >> 16, ((state->src_x & 0xffff) * 15625) >> 10,

				 state->src_y >> 16, ((state->src_y & 0xffff) * 15625) >> 10,

				 state->fb->width, state->fb->height);

				 new_plane_state->src_w >> 16,

				 ((new_plane_state->src_w & 0xffff) * 15625) >> 10,

				 new_plane_state->src_h >> 16,

				 ((new_plane_state->src_h & 0xffff) * 15625) >> 10,

				 new_plane_state->src_x >> 16,

				 ((new_plane_state->src_x & 0xffff) * 15625) >> 10,

				 new_plane_state->src_y >> 16,

				 ((new_plane_state->src_y & 0xffff) * 15625) >> 10,

				 fb->width, fb->height);

		return -ENOSPC;

	}



	if (plane_switching_crtc(state->state, plane, state)) {

	if (plane_switching_crtc(old_plane_state, new_plane_state)) {

		DRM_DEBUG_ATOMIC("[PLANE:%d:%s] switching CRTC directly\n",

				 plane->base.id, plane->name);

		return -EINVAL;
@@ -961,7 +967,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) 
	struct drm_device *dev = state->dev;

	struct drm_mode_config *config = &dev->mode_config;

	struct drm_plane *plane;

	struct drm_plane_state *plane_state;

	struct drm_plane_state *old_plane_state;

	struct drm_plane_state *new_plane_state;

	struct drm_crtc *crtc;

	struct drm_crtc_state *old_crtc_state;

	struct drm_crtc_state *new_crtc_state;
@@ -971,8 +978,8 @@ int drm_atomic_check_only(struct drm_atomic_state *state) 


	DRM_DEBUG_ATOMIC("checking %p\n", state);



	for_each_new_plane_in_state(state, plane, plane_state, i) {

		ret = drm_atomic_plane_check(plane, plane_state);

	for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {

		ret = drm_atomic_plane_check(old_plane_state, new_plane_state);

		if (ret) {

			DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check failed\n",

					 plane->base.id, plane->name);