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

[Mahesh Kumar <mahesh1.kumar@intel.com>]

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)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx