| Message ID | 20181115103721.25601-5-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Allow scaling on cursor planes | expand |
Boris Brezillon <boris.brezillon@bootlin.com> writes: > vc4_plane_atomic_async_check() was only based on the > state->{crtc,src}_{w,h} which was fine since scaling was not allowed on > the cursor plane. > > We are about to change that to properly support underscan, and, in order > to make the async check more reliable, we call vc4_plane_mode_set() > from there and check that only the pos0, pos2 and ptr0 entries in the > dlist have changed. > > In vc4_plane_atomic_async_update(), we no longer call > vc4_plane_atomic_check() since vc4_plane_mode_set() has already been > called in vc4_plane_atomic_async_check(), and we don't need to allocate > a new LBM region (we reuse the one from the current state). > > Note that we now have to manually update each field of the current > plane state since it's no longer updated in place (not sure we have > to sync all of them, but it's harmless if we do). > We also drop the vc4_plane_async_set_fb() call (ptr0 dlist entry has > been properly updated in vc4_plane_mode_set()) > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/gpu/drm/vc4/vc4_plane.c | 87 +++++++++++++++++++++++++-------- > 1 file changed, 66 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 09c7478b095b..31c7b63dd723 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -895,30 +895,50 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, > { > struct vc4_plane_state *vc4_state, *new_vc4_state; > > - if (plane->state->fb != state->fb) { > - vc4_plane_async_set_fb(plane, state->fb); > - drm_atomic_set_fb_for_plane(plane->state, state->fb); > - } > - > - /* Set the cursor's position on the screen. This is the > - * expected change from the drm_mode_cursor_universal() > - * helper. > - */ > + drm_atomic_set_fb_for_plane(plane->state, state->fb); > plane->state->crtc_x = state->crtc_x; > plane->state->crtc_y = state->crtc_y; > - > - /* Allow changing the start position within the cursor BO, if > - * that matters. > - */ > + plane->state->crtc_w = state->crtc_w; > + plane->state->crtc_h = state->crtc_h; > plane->state->src_x = state->src_x; > plane->state->src_y = state->src_y; > - > - /* Update the display list based on the new crtc_x/y. */ > - vc4_plane_atomic_check(plane, state); > + plane->state->src_w = state->src_w; > + plane->state->src_h = state->src_h; > + plane->state->src_h = state->src_h; > + plane->state->alpha = state->alpha; > + plane->state->pixel_blend_mode = state->pixel_blend_mode; > + plane->state->rotation = state->rotation; > + plane->state->zpos = state->zpos; > + plane->state->normalized_zpos = state->normalized_zpos; > + plane->state->color_encoding = state->color_encoding; > + plane->state->color_range = state->color_range; > + plane->state->src = state->src; > + plane->state->dst = state->dst; > + plane->state->visible = state->visible; > > new_vc4_state = to_vc4_plane_state(state); > vc4_state = to_vc4_plane_state(plane->state); > > + vc4_state->crtc_x = new_vc4_state->crtc_x; > + vc4_state->crtc_y = new_vc4_state->crtc_y; > + vc4_state->crtc_h = new_vc4_state->crtc_h; > + vc4_state->crtc_w = new_vc4_state->crtc_w; > + vc4_state->src_x = new_vc4_state->src_x; > + vc4_state->src_y = new_vc4_state->src_y; > + memcpy(vc4_state->src_w, new_vc4_state->src_w, > + sizeof(vc4_state->src_w)); > + memcpy(vc4_state->src_h, new_vc4_state->src_h, > + sizeof(vc4_state->src_h)); > + memcpy(vc4_state->x_scaling, new_vc4_state->x_scaling, > + sizeof(vc4_state->x_scaling)); > + memcpy(vc4_state->y_scaling, new_vc4_state->y_scaling, > + sizeof(vc4_state->y_scaling)); > + vc4_state->is_unity = new_vc4_state->is_unity; > + vc4_state->is_yuv = new_vc4_state->is_yuv; > + memcpy(vc4_state->offsets, new_vc4_state->offsets, > + sizeof(vc4_state->offsets)); > + vc4_state->needs_bg_fill = new_vc4_state->needs_bg_fill; This copying feels like a maintenance nightmare to me -- nothing's going to be testing async updates of each random bit of state, so if something new could change while passing atomic_async_check(), we're going to get it wrong. Any ideas for what we can do to handle that? > + > /* Update the current vc4_state pos0, pos2 and ptr0 dlist entries. */ > vc4_state->dlist[vc4_state->pos0_offset] = > new_vc4_state->dlist[vc4_state->pos0_offset]; > @@ -942,13 +962,38 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, > static int vc4_plane_atomic_async_check(struct drm_plane *plane, > struct drm_plane_state *state) > { > - /* No configuring new scaling in the fast path. */ > - if (plane->state->crtc_w != state->crtc_w || > - plane->state->crtc_h != state->crtc_h || > - plane->state->src_w != state->src_w || > - plane->state->src_h != state->src_h) > + struct vc4_plane_state *old_vc4_state, *new_vc4_state; > + int ret; > + u32 i; > + > + ret = vc4_plane_mode_set(plane, state); > + if (ret) > + return ret; > + > + old_vc4_state = to_vc4_plane_state(plane->state); > + new_vc4_state = to_vc4_plane_state(state); > + if (old_vc4_state->dlist_count != new_vc4_state->dlist_count || > + old_vc4_state->pos0_offset != new_vc4_state->pos0_offset || > + old_vc4_state->pos2_offset != new_vc4_state->pos2_offset || > + old_vc4_state->ptr0_offset != new_vc4_state->ptr0_offset || > + vc4_lbm_size(plane->state) != vc4_lbm_size(state)) > return -EINVAL; > > + /* Only pos0, pos2 and ptr0 DWORDS can be updated in an async update > + * if anything else has changed, fallback to a sync update. > + */ > + for (i = 0; i < new_vc4_state->dlist_count; i++) { > + if (i == new_vc4_state->pos0_offset || > + i == new_vc4_state->pos2_offset || > + i == new_vc4_state->ptr0_offset || > + (new_vc4_state->lbm_offset && > + i == new_vc4_state->lbm_offset)) > + continue; > + > + if (new_vc4_state->dlist[i] != old_vc4_state->dlist[i]) > + return -EINVAL; > + } > + I really like these new checks for whether we can do the async update, compared to my old ones!
On Thu, 15 Nov 2018 12:49:11 -0800 Eric Anholt <eric@anholt.net> wrote: > Boris Brezillon <boris.brezillon@bootlin.com> writes: > > > vc4_plane_atomic_async_check() was only based on the > > state->{crtc,src}_{w,h} which was fine since scaling was not allowed on > > the cursor plane. > > > > We are about to change that to properly support underscan, and, in order > > to make the async check more reliable, we call vc4_plane_mode_set() > > from there and check that only the pos0, pos2 and ptr0 entries in the > > dlist have changed. > > > > In vc4_plane_atomic_async_update(), we no longer call > > vc4_plane_atomic_check() since vc4_plane_mode_set() has already been > > called in vc4_plane_atomic_async_check(), and we don't need to allocate > > a new LBM region (we reuse the one from the current state). > > > > Note that we now have to manually update each field of the current > > plane state since it's no longer updated in place (not sure we have > > to sync all of them, but it's harmless if we do). > > We also drop the vc4_plane_async_set_fb() call (ptr0 dlist entry has > > been properly updated in vc4_plane_mode_set()) > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/gpu/drm/vc4/vc4_plane.c | 87 +++++++++++++++++++++++++-------- > > 1 file changed, 66 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > > index 09c7478b095b..31c7b63dd723 100644 > > --- a/drivers/gpu/drm/vc4/vc4_plane.c > > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > > @@ -895,30 +895,50 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, > > { > > struct vc4_plane_state *vc4_state, *new_vc4_state; > > > > - if (plane->state->fb != state->fb) { > > - vc4_plane_async_set_fb(plane, state->fb); > > - drm_atomic_set_fb_for_plane(plane->state, state->fb); > > - } > > - > > - /* Set the cursor's position on the screen. This is the > > - * expected change from the drm_mode_cursor_universal() > > - * helper. > > - */ > > + drm_atomic_set_fb_for_plane(plane->state, state->fb); > > plane->state->crtc_x = state->crtc_x; > > plane->state->crtc_y = state->crtc_y; > > - > > - /* Allow changing the start position within the cursor BO, if > > - * that matters. > > - */ > > + plane->state->crtc_w = state->crtc_w; > > + plane->state->crtc_h = state->crtc_h; > > plane->state->src_x = state->src_x; > > plane->state->src_y = state->src_y; > > - > > - /* Update the display list based on the new crtc_x/y. */ > > - vc4_plane_atomic_check(plane, state); > > + plane->state->src_w = state->src_w; > > + plane->state->src_h = state->src_h; > > + plane->state->src_h = state->src_h; > > + plane->state->alpha = state->alpha; > > + plane->state->pixel_blend_mode = state->pixel_blend_mode; > > + plane->state->rotation = state->rotation; > > + plane->state->zpos = state->zpos; > > + plane->state->normalized_zpos = state->normalized_zpos; > > + plane->state->color_encoding = state->color_encoding; > > + plane->state->color_range = state->color_range; > > + plane->state->src = state->src; > > + plane->state->dst = state->dst; > > + plane->state->visible = state->visible; > > > > new_vc4_state = to_vc4_plane_state(state); > > vc4_state = to_vc4_plane_state(plane->state); > > > > + vc4_state->crtc_x = new_vc4_state->crtc_x; > > + vc4_state->crtc_y = new_vc4_state->crtc_y; > > + vc4_state->crtc_h = new_vc4_state->crtc_h; > > + vc4_state->crtc_w = new_vc4_state->crtc_w; > > + vc4_state->src_x = new_vc4_state->src_x; > > + vc4_state->src_y = new_vc4_state->src_y; > > + memcpy(vc4_state->src_w, new_vc4_state->src_w, > > + sizeof(vc4_state->src_w)); > > + memcpy(vc4_state->src_h, new_vc4_state->src_h, > > + sizeof(vc4_state->src_h)); > > + memcpy(vc4_state->x_scaling, new_vc4_state->x_scaling, > > + sizeof(vc4_state->x_scaling)); > > + memcpy(vc4_state->y_scaling, new_vc4_state->y_scaling, > > + sizeof(vc4_state->y_scaling)); > > + vc4_state->is_unity = new_vc4_state->is_unity; > > + vc4_state->is_yuv = new_vc4_state->is_yuv; > > + memcpy(vc4_state->offsets, new_vc4_state->offsets, > > + sizeof(vc4_state->offsets)); > > + vc4_state->needs_bg_fill = new_vc4_state->needs_bg_fill; > > This copying feels like a maintenance nightmare to me -- nothing's going > to be testing async updates of each random bit of state, so if something > new could change while passing atomic_async_check(), we're going to get > it wrong. Yeah, I don't like it either. I'd definitely prefer if states could be swapped somehow, but then you have the problem of migrating some of the resources from the old state to the new one (the most obvious one being the LBM drm_mm_node object which is already inserted in a list, but I'm pretty we have the same issue with other fields too). > > Any ideas for what we can do to handle that? Nope. > > > + > > /* Update the current vc4_state pos0, pos2 and ptr0 dlist entries. */ > > vc4_state->dlist[vc4_state->pos0_offset] = > > new_vc4_state->dlist[vc4_state->pos0_offset]; > > @@ -942,13 +962,38 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, > > static int vc4_plane_atomic_async_check(struct drm_plane *plane, > > struct drm_plane_state *state) > > { > > - /* No configuring new scaling in the fast path. */ > > - if (plane->state->crtc_w != state->crtc_w || > > - plane->state->crtc_h != state->crtc_h || > > - plane->state->src_w != state->src_w || > > - plane->state->src_h != state->src_h) > > + struct vc4_plane_state *old_vc4_state, *new_vc4_state; > > + int ret; > > + u32 i; > > + > > + ret = vc4_plane_mode_set(plane, state); > > + if (ret) > > + return ret; > > + > > + old_vc4_state = to_vc4_plane_state(plane->state); > > + new_vc4_state = to_vc4_plane_state(state); > > + if (old_vc4_state->dlist_count != new_vc4_state->dlist_count || > > + old_vc4_state->pos0_offset != new_vc4_state->pos0_offset || > > + old_vc4_state->pos2_offset != new_vc4_state->pos2_offset || > > + old_vc4_state->ptr0_offset != new_vc4_state->ptr0_offset || > > + vc4_lbm_size(plane->state) != vc4_lbm_size(state)) > > return -EINVAL; > > > > + /* Only pos0, pos2 and ptr0 DWORDS can be updated in an async update > > + * if anything else has changed, fallback to a sync update. > > + */ > > + for (i = 0; i < new_vc4_state->dlist_count; i++) { > > + if (i == new_vc4_state->pos0_offset || > > + i == new_vc4_state->pos2_offset || > > + i == new_vc4_state->ptr0_offset || > > + (new_vc4_state->lbm_offset && > > + i == new_vc4_state->lbm_offset)) > > + continue; > > + > > + if (new_vc4_state->dlist[i] != old_vc4_state->dlist[i]) > > + return -EINVAL; > > + } > > + > > I really like these new checks for whether we can do the async update, > compared to my old ones! Yes. I'm pretty sure we can allow async updates when other of those dlist entries change, but I wanted to be on the safe side.
Boris Brezillon <boris.brezillon@bootlin.com> writes: > On Thu, 15 Nov 2018 12:49:11 -0800 > Eric Anholt <eric@anholt.net> wrote: > >> Boris Brezillon <boris.brezillon@bootlin.com> writes: >> >> > vc4_plane_atomic_async_check() was only based on the >> > state->{crtc,src}_{w,h} which was fine since scaling was not allowed on >> > the cursor plane. >> > >> > We are about to change that to properly support underscan, and, in order >> > to make the async check more reliable, we call vc4_plane_mode_set() >> > from there and check that only the pos0, pos2 and ptr0 entries in the >> > dlist have changed. >> > >> > In vc4_plane_atomic_async_update(), we no longer call >> > vc4_plane_atomic_check() since vc4_plane_mode_set() has already been >> > called in vc4_plane_atomic_async_check(), and we don't need to allocate >> > a new LBM region (we reuse the one from the current state). >> > >> > Note that we now have to manually update each field of the current >> > plane state since it's no longer updated in place (not sure we have >> > to sync all of them, but it's harmless if we do). >> > We also drop the vc4_plane_async_set_fb() call (ptr0 dlist entry has >> > been properly updated in vc4_plane_mode_set()) >> > >> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> >> > --- >> > drivers/gpu/drm/vc4/vc4_plane.c | 87 +++++++++++++++++++++++++-------- >> > 1 file changed, 66 insertions(+), 21 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c >> > index 09c7478b095b..31c7b63dd723 100644 >> > --- a/drivers/gpu/drm/vc4/vc4_plane.c >> > +++ b/drivers/gpu/drm/vc4/vc4_plane.c >> > @@ -895,30 +895,50 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, >> > { >> > struct vc4_plane_state *vc4_state, *new_vc4_state; >> > >> > - if (plane->state->fb != state->fb) { >> > - vc4_plane_async_set_fb(plane, state->fb); >> > - drm_atomic_set_fb_for_plane(plane->state, state->fb); >> > - } >> > - >> > - /* Set the cursor's position on the screen. This is the >> > - * expected change from the drm_mode_cursor_universal() >> > - * helper. >> > - */ >> > + drm_atomic_set_fb_for_plane(plane->state, state->fb); >> > plane->state->crtc_x = state->crtc_x; >> > plane->state->crtc_y = state->crtc_y; >> > - >> > - /* Allow changing the start position within the cursor BO, if >> > - * that matters. >> > - */ >> > + plane->state->crtc_w = state->crtc_w; >> > + plane->state->crtc_h = state->crtc_h; >> > plane->state->src_x = state->src_x; >> > plane->state->src_y = state->src_y; >> > - >> > - /* Update the display list based on the new crtc_x/y. */ >> > - vc4_plane_atomic_check(plane, state); >> > + plane->state->src_w = state->src_w; >> > + plane->state->src_h = state->src_h; >> > + plane->state->src_h = state->src_h; >> > + plane->state->alpha = state->alpha; >> > + plane->state->pixel_blend_mode = state->pixel_blend_mode; >> > + plane->state->rotation = state->rotation; >> > + plane->state->zpos = state->zpos; >> > + plane->state->normalized_zpos = state->normalized_zpos; >> > + plane->state->color_encoding = state->color_encoding; >> > + plane->state->color_range = state->color_range; >> > + plane->state->src = state->src; >> > + plane->state->dst = state->dst; >> > + plane->state->visible = state->visible; >> > >> > new_vc4_state = to_vc4_plane_state(state); >> > vc4_state = to_vc4_plane_state(plane->state); >> > >> > + vc4_state->crtc_x = new_vc4_state->crtc_x; >> > + vc4_state->crtc_y = new_vc4_state->crtc_y; >> > + vc4_state->crtc_h = new_vc4_state->crtc_h; >> > + vc4_state->crtc_w = new_vc4_state->crtc_w; >> > + vc4_state->src_x = new_vc4_state->src_x; >> > + vc4_state->src_y = new_vc4_state->src_y; >> > + memcpy(vc4_state->src_w, new_vc4_state->src_w, >> > + sizeof(vc4_state->src_w)); >> > + memcpy(vc4_state->src_h, new_vc4_state->src_h, >> > + sizeof(vc4_state->src_h)); >> > + memcpy(vc4_state->x_scaling, new_vc4_state->x_scaling, >> > + sizeof(vc4_state->x_scaling)); >> > + memcpy(vc4_state->y_scaling, new_vc4_state->y_scaling, >> > + sizeof(vc4_state->y_scaling)); >> > + vc4_state->is_unity = new_vc4_state->is_unity; >> > + vc4_state->is_yuv = new_vc4_state->is_yuv; >> > + memcpy(vc4_state->offsets, new_vc4_state->offsets, >> > + sizeof(vc4_state->offsets)); >> > + vc4_state->needs_bg_fill = new_vc4_state->needs_bg_fill; >> >> This copying feels like a maintenance nightmare to me -- nothing's going >> to be testing async updates of each random bit of state, so if something >> new could change while passing atomic_async_check(), we're going to get >> it wrong. > > Yeah, I don't like it either. I'd definitely prefer if states could be > swapped somehow, but then you have the problem of migrating some of the > resources from the old state to the new one (the most obvious one being > the LBM drm_mm_node object which is already inserted in a list, but > I'm pretty we have the same issue with other fields too). > >> >> Any ideas for what we can do to handle that? > > Nope. I think I'm less concerned than I was the first time around. Realistically, potential new fields won't be changing anyway. If they did, they'd have an effect on the dlists other than position/pointer, and that would make them fail the async check. So, while I have some reservations, I'm also unhappy with my solution too, and I think yours has benefits that outweigh the cost of the code in question here. Reviewed-by: Eric Anholt <eric@anholt.net>
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 09c7478b095b..31c7b63dd723 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -895,30 +895,50 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, { struct vc4_plane_state *vc4_state, *new_vc4_state; - if (plane->state->fb != state->fb) { - vc4_plane_async_set_fb(plane, state->fb); - drm_atomic_set_fb_for_plane(plane->state, state->fb); - } - - /* Set the cursor's position on the screen. This is the - * expected change from the drm_mode_cursor_universal() - * helper. - */ + drm_atomic_set_fb_for_plane(plane->state, state->fb); plane->state->crtc_x = state->crtc_x; plane->state->crtc_y = state->crtc_y; - - /* Allow changing the start position within the cursor BO, if - * that matters. - */ + plane->state->crtc_w = state->crtc_w; + plane->state->crtc_h = state->crtc_h; plane->state->src_x = state->src_x; plane->state->src_y = state->src_y; - - /* Update the display list based on the new crtc_x/y. */ - vc4_plane_atomic_check(plane, state); + plane->state->src_w = state->src_w; + plane->state->src_h = state->src_h; + plane->state->src_h = state->src_h; + plane->state->alpha = state->alpha; + plane->state->pixel_blend_mode = state->pixel_blend_mode; + plane->state->rotation = state->rotation; + plane->state->zpos = state->zpos; + plane->state->normalized_zpos = state->normalized_zpos; + plane->state->color_encoding = state->color_encoding; + plane->state->color_range = state->color_range; + plane->state->src = state->src; + plane->state->dst = state->dst; + plane->state->visible = state->visible; new_vc4_state = to_vc4_plane_state(state); vc4_state = to_vc4_plane_state(plane->state); + vc4_state->crtc_x = new_vc4_state->crtc_x; + vc4_state->crtc_y = new_vc4_state->crtc_y; + vc4_state->crtc_h = new_vc4_state->crtc_h; + vc4_state->crtc_w = new_vc4_state->crtc_w; + vc4_state->src_x = new_vc4_state->src_x; + vc4_state->src_y = new_vc4_state->src_y; + memcpy(vc4_state->src_w, new_vc4_state->src_w, + sizeof(vc4_state->src_w)); + memcpy(vc4_state->src_h, new_vc4_state->src_h, + sizeof(vc4_state->src_h)); + memcpy(vc4_state->x_scaling, new_vc4_state->x_scaling, + sizeof(vc4_state->x_scaling)); + memcpy(vc4_state->y_scaling, new_vc4_state->y_scaling, + sizeof(vc4_state->y_scaling)); + vc4_state->is_unity = new_vc4_state->is_unity; + vc4_state->is_yuv = new_vc4_state->is_yuv; + memcpy(vc4_state->offsets, new_vc4_state->offsets, + sizeof(vc4_state->offsets)); + vc4_state->needs_bg_fill = new_vc4_state->needs_bg_fill; + /* Update the current vc4_state pos0, pos2 and ptr0 dlist entries. */ vc4_state->dlist[vc4_state->pos0_offset] = new_vc4_state->dlist[vc4_state->pos0_offset]; @@ -942,13 +962,38 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, static int vc4_plane_atomic_async_check(struct drm_plane *plane, struct drm_plane_state *state) { - /* No configuring new scaling in the fast path. */ - if (plane->state->crtc_w != state->crtc_w || - plane->state->crtc_h != state->crtc_h || - plane->state->src_w != state->src_w || - plane->state->src_h != state->src_h) + struct vc4_plane_state *old_vc4_state, *new_vc4_state; + int ret; + u32 i; + + ret = vc4_plane_mode_set(plane, state); + if (ret) + return ret; + + old_vc4_state = to_vc4_plane_state(plane->state); + new_vc4_state = to_vc4_plane_state(state); + if (old_vc4_state->dlist_count != new_vc4_state->dlist_count || + old_vc4_state->pos0_offset != new_vc4_state->pos0_offset || + old_vc4_state->pos2_offset != new_vc4_state->pos2_offset || + old_vc4_state->ptr0_offset != new_vc4_state->ptr0_offset || + vc4_lbm_size(plane->state) != vc4_lbm_size(state)) return -EINVAL; + /* Only pos0, pos2 and ptr0 DWORDS can be updated in an async update + * if anything else has changed, fallback to a sync update. + */ + for (i = 0; i < new_vc4_state->dlist_count; i++) { + if (i == new_vc4_state->pos0_offset || + i == new_vc4_state->pos2_offset || + i == new_vc4_state->ptr0_offset || + (new_vc4_state->lbm_offset && + i == new_vc4_state->lbm_offset)) + continue; + + if (new_vc4_state->dlist[i] != old_vc4_state->dlist[i]) + return -EINVAL; + } + return 0; }
vc4_plane_atomic_async_check() was only based on the state->{crtc,src}_{w,h} which was fine since scaling was not allowed on the cursor plane. We are about to change that to properly support underscan, and, in order to make the async check more reliable, we call vc4_plane_mode_set() from there and check that only the pos0, pos2 and ptr0 entries in the dlist have changed. In vc4_plane_atomic_async_update(), we no longer call vc4_plane_atomic_check() since vc4_plane_mode_set() has already been called in vc4_plane_atomic_async_check(), and we don't need to allocate a new LBM region (we reuse the one from the current state). Note that we now have to manually update each field of the current plane state since it's no longer updated in place (not sure we have to sync all of them, but it's harmless if we do). We also drop the vc4_plane_async_set_fb() call (ptr0 dlist entry has been properly updated in vc4_plane_mode_set()) Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/gpu/drm/vc4/vc4_plane.c | 87 +++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 21 deletions(-)