Re: [PATCH v2 3/4] drm/imx: call drm_atomic_helper_commit_hw_done after drm_atomic_helper_wait_for_vblanks

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mon, 2017-02-27 at 17:25 +0100, Daniel Vetter wrote:
> On Mon, Feb 27, 2017 at 02:14:57PM +0100, Philipp Zabel wrote:
> > Disabling planes will consist of two steps as of the following patch.
> > First, the DP is asked to stop at the next vblank, and then, after the
> > vblank the associated IDMAC channel is idle and can be disabled.
> > To avoid further commits being awoken out of their wait for dependencies
> > too early, we should report commit_hw_done only after wait_for_vblanks.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/gpu/drm/imx/imx-drm-core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
> > index f562cb7964b08..1ed120c181724 100644
> > --- a/drivers/gpu/drm/imx/imx-drm-core.c
> > +++ b/drivers/gpu/drm/imx/imx-drm-core.c
> > @@ -169,10 +169,10 @@ static void imx_drm_atomic_commit_tail(struct drm_atomic_state *state)
> >  
> >  	drm_atomic_helper_commit_modeset_enables(dev, state);
> >  
> > -	drm_atomic_helper_commit_hw_done(state);
> > -
> >  	drm_atomic_helper_wait_for_vblanks(dev, state);
> >  
> > +	drm_atomic_helper_commit_hw_done(state);
> 
> That gives you a pretty good chance of halfing your refresh rate for async
> flips. At least if you're not super careful about things. Did you
> double-check this?
> -Daniel

I'm not used to the terminology, does async flips just mean
DRM_MODE_PAGE_FLIP_ASYNC? We can't support that at all, since the
hardware can't flip during hblank. We are not allowed to touch or even
look at the DMA channel parameter memory during the whole frame,
unfortunately.

With sync flips during vblank, which is the only thing we can support,
the only place waiting for the hw_done completion is
drm_atomic_helper_wait_for_dependencies, which goes on to wait for the
same commit's flip_done completion right afterwards. That is completed
by the same vblank IRQ that wait_for_vblanks is also waiting for.

So to be completely correct, we could report hw_done before
wait_for_vblanks only if there are no plane disables pending, and
afterwards otherwise, but I don't think this should make a difference
right now.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel