| Message ID | 20181115103721.25601-4-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: > We are about to use vc4_plane_mode_set() in the async check path, but > async check can decide that async update is not possible and force the > driver to fallback to a sync update. > > All the checks that have been done on the plane state during async check > stay valid, and checking it again is not necessary. Add a ->checked > field to vc4_plane_state, and use it to track the status of the state > (checked or not). > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++ > drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > index 9ed05fb61eb6..d1000c4805c2 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.h > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > @@ -370,6 +370,11 @@ struct vc4_plane_state { > * to enable background color fill. > */ > bool needs_bg_fill; > + > + /* Mark the state as checked. Useful to avoid checking it twice when > + * async update is not possible. > + */ > + bool checked; > }; Since this doesn't cover the whole atomic_check process, which won't have been called from async update, maybe rename to dlist_initialized or something?
On Thu, 15 Nov 2018 12:41:36 -0800 Eric Anholt <eric@anholt.net> wrote: > Boris Brezillon <boris.brezillon@bootlin.com> writes: > > > We are about to use vc4_plane_mode_set() in the async check path, but > > async check can decide that async update is not possible and force the > > driver to fallback to a sync update. > > > > All the checks that have been done on the plane state during async check > > stay valid, and checking it again is not necessary. Add a ->checked > > field to vc4_plane_state, and use it to track the status of the state > > (checked or not). > > > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > > --- > > drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++ > > drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > > index 9ed05fb61eb6..d1000c4805c2 100644 > > --- a/drivers/gpu/drm/vc4/vc4_drv.h > > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > > @@ -370,6 +370,11 @@ struct vc4_plane_state { > > * to enable background color fill. > > */ > > bool needs_bg_fill; > > + > > + /* Mark the state as checked. Useful to avoid checking it twice when > > + * async update is not possible. > > + */ > > + bool checked; > > }; > > Since this doesn't cover the whole atomic_check process, which won't > have been called from async update, maybe rename to dlist_initialized or > something? Do you mean that vc4_plane_mode_set() should be run again from the sync check path when async check failed, or is it just the name you don't like?
Boris Brezillon <boris.brezillon@bootlin.com> writes: > On Thu, 15 Nov 2018 12:41:36 -0800 > Eric Anholt <eric@anholt.net> wrote: > >> Boris Brezillon <boris.brezillon@bootlin.com> writes: >> >> > We are about to use vc4_plane_mode_set() in the async check path, but >> > async check can decide that async update is not possible and force the >> > driver to fallback to a sync update. >> > >> > All the checks that have been done on the plane state during async check >> > stay valid, and checking it again is not necessary. Add a ->checked >> > field to vc4_plane_state, and use it to track the status of the state >> > (checked or not). >> > >> > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> >> > --- >> > drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++ >> > drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ >> > 2 files changed, 15 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h >> > index 9ed05fb61eb6..d1000c4805c2 100644 >> > --- a/drivers/gpu/drm/vc4/vc4_drv.h >> > +++ b/drivers/gpu/drm/vc4/vc4_drv.h >> > @@ -370,6 +370,11 @@ struct vc4_plane_state { >> > * to enable background color fill. >> > */ >> > bool needs_bg_fill; >> > + >> > + /* Mark the state as checked. Useful to avoid checking it twice when >> > + * async update is not possible. >> > + */ >> > + bool checked; >> > }; >> >> Since this doesn't cover the whole atomic_check process, which won't >> have been called from async update, maybe rename to dlist_initialized or >> something? > > Do you mean that vc4_plane_mode_set() should be run again from the sync > check path when async check failed, or is it just the name you don't > like? Just the name, s/checked/dlist_initialized/ (or something)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 9ed05fb61eb6..d1000c4805c2 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -370,6 +370,11 @@ struct vc4_plane_state { * to enable background color fill. */ bool needs_bg_fill; + + /* Mark the state as checked. Useful to avoid checking it twice when + * async update is not possible. + */ + bool checked; }; static inline struct vc4_plane_state * diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 392a51f2bf8f..09c7478b095b 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -154,6 +154,7 @@ static struct drm_plane_state *vc4_plane_duplicate_state(struct drm_plane *plane return NULL; memset(&vc4_state->lbm, 0, sizeof(vc4_state->lbm)); + vc4_state->checked = 0; __drm_atomic_helper_plane_duplicate_state(plane, &vc4_state->base); @@ -510,6 +511,8 @@ static int vc4_plane_mode_set(struct drm_plane *plane, u32 hvs_format = format->hvs; int ret, i; + if (vc4_state->checked) + return 0; ret = vc4_plane_setup_clipping_and_scaling(state); if (ret) @@ -792,6 +795,13 @@ static int vc4_plane_mode_set(struct drm_plane *plane, vc4_state->needs_bg_fill = fb->format->has_alpha || !covers_screen || state->alpha != DRM_BLEND_ALPHA_OPAQUE; + /* Flag the state as already checked to avoid checking it twice in case + * the async update check already called vc4_plane_mode_set() and + * decided to fallback to sync update because async update was not + * possible. + */ + vc4_state->checked = 1; + return 0; }
We are about to use vc4_plane_mode_set() in the async check path, but async check can decide that async update is not possible and force the driver to fallback to a sync update. All the checks that have been done on the plane state during async check stay valid, and checking it again is not necessary. Add a ->checked field to vc4_plane_state, and use it to track the status of the state (checked or not). Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- drivers/gpu/drm/vc4/vc4_drv.h | 5 +++++ drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ 2 files changed, 15 insertions(+)