[Intel-gfx] [PATCH 04/26] drm/rcar-du: Use for_each_*_in_state

[Laurent Pinchart]

Hi Daniel,

On Friday 03 Jun 2016 11:45:56 Daniel Vetter wrote:
> On Fri, Jun 3, 2016 at 11:40 AM, Laurent Pinchart wrote:
> > On Friday 03 Jun 2016 08:55:36 Daniel Vetter wrote:
> >> On Fri, Jun 03, 2016 at 01:54:44AM +0300, Laurent Pinchart wrote:
> >>> On Monday 30 May 2016 16:54:10 Daniel Vetter wrote:
> >>>> On Mon, May 30, 2016 at 11:58:27AM +0200, Maarten Lankhorst wrote:
> >>>>> Op 30-05-16 om 11:18 schreef Laurent Pinchart:
> >>>>>> Hi Daniel,
> >>>>>> 
> >>>>>> Thank you for the patch.
> >>>>>> 
> >>>>>> This looks good to me as the resulting code is mostly similar.
> >>>>>> However, the for_each_*_in_state macros end with an for_each_if()
> >>>>>> that tests if the object's state is NULL, which isn't present in
> >>>>>> this code. I'm wondering whether that was an oversight on my side
> >>>>>> possibly leading to a crash when dereferencing a NULL state, or an
> >>>>>> unneeded check in the macros. Can atomic_state->*_states[i] be NULL
> >>>>>> if atomic_state->*[i] is not NULL ?
> >>>>> 
> >>>>> Not in any normal case.
> >>>> 
> >>>> Yeah, the drm_atomic_get_*_state functions only ever fill in both of
> >>>> neither. If this gets out of sync it's a bug ;-)
> >>> 
> >>> Should the check be removed then ? Or replaced by a WARN_ON() ?
> >> 
> >> In all the places I converted here I nuked those checks, since they moved
> >> into the loop now. Not sure what checks you're talking about.
> > 
> > I'm talking about the for_each_if() check inside the for_each_*_in_state
> > macros.
> 
> The rule is drm_atomic_state->plane[i] != NULL iff
> drm_atomic_state->plane_state != NULL. So you can check either of them
> for the same result. But you still need to check one of them,
> otherwise all the loops in drivers and helpers will oops. Not sure why
> you want to remove that check, your driver had the equivalent (which I
> removed) too.

My bad, I had misread the for_each_*_in_state macros and thought they were 
checking both. Sorry for the noise.

-- 
Regards,

Laurent Pinchart