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.