Re: [PATCH 4/4] drm/atmel-hlcdc: do not immediately disable planes, wait for next frame

[Boris Brezillon]

On Thu, 10 Jan 2019 18:51:21 +0000
Peter Rosin <peda@xxxxxxxxxx> wrote:

> On 2019-01-10 18:29, Boris Brezillon wrote:
> > On Thu, 10 Jan 2019 15:10:48 +0000
> > Peter Rosin <peda@xxxxxxxxxx> wrote:
> >   
> >> The A2Q and UPDATE bits have no effect in the channel disable registers.
> >> However, since they are present, assume that the intention is to disable
> >> planes, not immediately as indicated by the RST bit, but on the next
> >> frame shift since that is what A2Q and UPDATE means in the channel enable
> >> registers.
> >>
> >> Disabling the plane on the next frame shift is done with the EN bit,
> >> so use that.  
> > 
> > It's been a long time, but I think I had a good reason for forcing a
> > reset. IIRC, when you don't do that and the CRTC is disabled before the
> > plane, the EN bit stays around, and next time you queue a plane update,
> > you'll start with an invalid buf pointer.  
> 
> It might be possible to clear the EN bit in ...CHDR before enabling the
> plane in ...CHER. Or is that too late?

I think I tried that, but I'm not sure (BTW, this change was done in
bd4248bb5e8b ("drm: atmel-hlcdc: reset layer A2Q and UPDATE bits when
disabling it")). Anyway, I'm not even sure this is still needed now
that atomic updates have a wait_for_flip_done/vblank() in the commit
path.

> But this patch is not overly
> important, I just wanted to avoid the resulting "black hole" when the
> plane DMA is disabled mid-frame. But disabling planes is probably not
> something that happens frequently and will perhaps not be noticed at
> all...

Okay. Other patches look good to me, I'm just waiting for Nicolas
feedback before applying them.