[PATCH] drm/atomic: Add new reverse iterator over all plane state

[S, Shirish]

Hi Alex,

Have resent the V2 with R-B of Daniel.

Regards,
Shirish S


-----Original Message-----
From: Alex Deucher [mailto:alexdeucher at gmail.com] 
Sent: Tuesday, March 6, 2018 11:01 PM
To: Vishwakarma, Pratik <Pratik.Vishwakarma at amd.com>
Cc: Daniel Vetter <daniel at ffwll.ch>; Deucher, Alexander <Alexander.Deucher at amd.com>; amd-gfx at lists.freedesktop.org; Maling list - DRI developers <dri-devel at lists.freedesktop.org>; S, Shirish <Shirish.S at amd.com>
Subject: Re: [PATCH] drm/atomic: Add new reverse iterator over all plane state

On Tue, Mar 6, 2018 at 5:52 AM, Vishwakarma, Pratik <Pratik.Vishwakarma at amd.com> wrote:
> Hi Daniel,
>
> I have checked make htmldocs on v2 of this patch. I have attached output drm-kms.html on that thread.
> No indentation issue is observed. Attached again for reference.
> Can you please provide RB on that?

How did you send the patch?  I can't get V2 to apply.  The patch is mangled.  Please use git-send-email if you didn't before.

Alex

>
> Regards
> Pratik
>
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf 
> Of Daniel Vetter
> Sent: Tuesday, March 6, 2018 3:36 PM
> To: Alex Deucher <alexdeucher at gmail.com>
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; 
> amd-gfx at lists.freedesktop.org; Maling list - DRI developers 
> <dri-devel at lists.freedesktop.org>; S, Shirish <Shirish.S at amd.com>
> Subject: Re: [PATCH] drm/atomic: Add new reverse iterator over all 
> plane state
>
> On Wed, Feb 28, 2018 at 09:26:26AM -0500, Alex Deucher wrote:
>> + dri-devel
>>
>>
>> On Wed, Feb 28, 2018 at 4:33 AM, S, Shirish <Shirish.S at amd.com> wrote:
>> > From: Shirish S <shirish.s at amd.com>
>> >
>> > Add reverse iterator "for_each_oldnew_plane_in_state_reverse" to 
>> > complement "for_each_oldnew_plane_in_state" way of reading plane 
>> > states.
>> >
>> > The plane states are required to be read in reverse order for 
>> > amdgpu, as the z order convention followed in linux is opposite to 
>> > how the planes are supposed to be presented to DC engine, which is 
>> > in common to both windows and linux.
>> >
>> > Signed-off-by: Shirish S <shirish.s at amd.com>
>> > Signed-off-by: Pratik Vishwakarma <Pratik.Vishwakarma at amd.com>
>
> Makes sense.
>> > ---
>> >  include/drm/drm_atomic.h | 22 ++++++++++++++++++++++
>> >  1 file changed, 22 insertions(+)
>> >
>> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h 
>> > index cf13842..b947930 100644
>> > --- a/include/drm/drm_atomic.h
>> > +++ b/include/drm/drm_atomic.h
>> > @@ -754,6 +754,28 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
>> >                               (new_plane_state) = 
>> > (__state)->planes[__i].new_state, 1))
>> >
>> >  /**
>> > + * for_each_oldnew_plane_in_state_reverse - iterate over all 
>> > + planes in an atomic
>> > + * update in reverse order
>
> Are you sure this renders correctly in kernel-doc? Iirc you have to indent the continuation line.
>
> Assuming this is fixed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
>> > + * @__state: &struct drm_atomic_state pointer
>> > + * @plane: &struct drm_plane iteration cursor
>> > + * @old_plane_state: &struct drm_plane_state iteration cursor for 
>> > +the old state
>> > + * @new_plane_state: &struct drm_plane_state iteration cursor for 
>> > +the new state
>> > + * @__i: int iteration cursor, for macro-internal use
>> > + *
>> > + * This iterates over all planes in an atomic update in reverse 
>> > +order,
>> > + * tracking both old and  new state. This is useful in places 
>> > +where the
>> > + * state delta needs to be considered, for example in atomic check functions.
>> > + */
>> > +#define for_each_oldnew_plane_in_state_reverse(__state, plane, old_plane_state, new_plane_state, __i) \
>> > +       (for ((__i) = ((__state)->dev->mode_config.num_total_plane - 1);        \
>> > +            (__i) >= 0;                                                \
>> > +            (__i)--)                                                   \
>> > +               for_each_if ((__state)->planes[__i].ptr &&              \
>> > +                            ((plane) = (__state)->planes[__i].ptr,     \
>> > +                             (old_plane_state) = (__state)->planes[__i].old_state,\
>> > +                             (new_plane_state) = 
>> > +(__state)->planes[__i].new_state, 1)))
>> > +
>> > +/**
>> >   * for_each_old_plane_in_state - iterate over all planes in an atomic update
>> >   * @__state: &struct drm_atomic_state pointer
>> >   * @plane: &struct drm_plane iteration cursor
>> > --
>> > 2.7.4
>> >
>> > _______________________________________________
>> > amd-gfx mailing list
>> > amd-gfx at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx