Re: [PATCH v4 1/5] drm/atomic: Fix atomic helpers to use the new iterator macros, v2.

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Maarten,

Thank you for the patch.

On Thursday 16 Feb 2017 15:47:06 Maarten Lankhorst wrote:
> There are new iterator macros that annotate whether the new or old
> state should be used. This is better than using a state that depends on
> whether it's called before or after swap. For clarity, also rename the
> variables from $obj_state to (old,new)_$obj_state as well.
> 
> Changes since v1:
> - Use old/new_*_state for variable names as much as possible. (pinchartl)
> - Expand commit message.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 431 +++++++++++++++++----------------
>  1 file changed, 222 insertions(+), 209 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index 9203f3e933f7..7d432d9a18cf
> 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c

[snip]

> @@ -1929,11 +1926,21 @@ void drm_atomic_helper_cleanup_planes(struct
> drm_device *dev, struct drm_atomic_state *old_state)
>  {
>  	struct drm_plane *plane;
> -	struct drm_plane_state *plane_state;
> +	struct drm_plane_state *old_plane_state, *new_plane_state;
>  	int i;
> 
> -	for_each_plane_in_state(old_state, plane, plane_state, i) {
> +	for_each_oldnew_plane_in_state(old_state, plane, old_plane_state,
> new_plane_state, i) { const struct drm_plane_helper_funcs *funcs;
> +		struct drm_plane_state *plane_state;
> +
> +		/*
> +		 * This might be called before swapping when commit is 
aborted,
> +		 * in which case we have to free the new state.

s/free/cleanup/

Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

You will however need to rebase the series on top of the latest drm-misc as it 
conflicts (at compile time) with

commit 40ee6fbef75fe6452dc9e69e6f9f1a2c7808ed67
Author: Manasi Navare <manasi.d.navare@intel.com>
Date:   Fri Dec 16 12:29:06 2016 +0200

    drm: Add a new connector atomic property for link status

> +		 */
> +		if (old_plane_state == plane->state)
> +			plane_state = new_plane_state;
> +		else
> +			plane_state = old_plane_state;
> 
>  		funcs = plane->helper_private;
> 

[snip]

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel