Re: [PATCH 3/8] drm/i915: Use new atomic helpers in intel_plane_atomic_check

Mahesh Kumar
2017-08-01T07:52:55Z

Hi,

On Thursday 20 July 2017 06:45 PM, Maarten Lankhorst wrote:
> Remove the use of plane->state and drm_atomic_get_existing_state,
> instead use the new helpers, and also add
> intel_atomic_get_new_crtc_state as it's needed.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_atomic_plane.c | 22 +++++++++++++---------
> drivers/gpu/drm/i915/intel_drv.h | 14 ++++++++++++++
> 2 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index ee76fab7bb6f..1081d0b63b6e 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -198,12 +198,17 @@ int intel_plane_atomic_check_with_state(struct intel_crtc_state *crtc_state,
> }
>
> static int intel_plane_atomic_check(struct drm_plane *plane,
> - struct drm_plane_state *state)
> + struct drm_plane_state *new_plane_state)
> {
> - struct drm_crtc *crtc = state->crtc;
> - struct drm_crtc_state *drm_crtc_state;
> + struct drm_atomic_state *state = new_plane_state->state;
> + struct intel_crtc *crtc;
> + struct intel_crtc_state *new_crtc_state;
> + struct drm_plane_state *old_plane_state;
> + struct intel_plane_state *new_intel_plane_state;
>
> - crtc = crtc ? crtc : plane->state->crtc;
> + old_plane_state = drm_atomic_get_old_plane_state(state, plane);
> +
> + crtc = to_intel_crtc(new_plane_state->crtc ?: old_plane_state->crtc);
>
> /*
> * Both crtc and plane->crtc could be NULL if we're updating a
you may want to update comment as well for better understanding. :)
> @@ -214,12 +219,11 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
> if (!crtc)
> return 0;
>
> - drm_crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
> - if (WARN_ON(!drm_crtc_state))
> - return -EINVAL;
> + new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
> + new_intel_plane_state = to_intel_plane_state(new_plane_state);
>
> - return intel_plane_atomic_check_with_state(to_intel_crtc_state(drm_crtc_state),
> - to_intel_plane_state(state));
> + return intel_plane_atomic_check_with_state(new_crtc_state,
> + new_intel_plane_state);
> }
>
> static void intel_plane_atomic_update(struct drm_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3723ee443faa..98851ed5ec2b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1957,6 +1957,20 @@ intel_atomic_get_old_crtc_state(struct drm_atomic_state *state,
> return NULL;
> }
>
> +static inline struct intel_crtc_state *
> +intel_atomic_get_new_crtc_state(struct drm_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + struct drm_crtc_state *crtc_state;
> +
> + crtc_state = drm_atomic_get_new_crtc_state(state, &crtc->base);
> +
> + if (crtc_state)
> + return to_intel_crtc_state(crtc_state);
> + else
> + return NULL;
> +}
> +
With previous patch comment addressed, This wrapper will be already
available.
otherwise patch looks ok to me.

-Mahesh
> static inline struct intel_plane_state *
> intel_atomic_get_new_plane_state(struct drm_atomic_state *state,
> struct intel_plane *plane)