Am Sonntag, 2. Juni 2024, 05:35:36 CEST schrieb Val Packett:
>
> On Mon, May 27 2024 at 20:11:49 -03:00:00, Val Packett
> <val@xxxxxxxxxxxx> wrote:
> > The RK3066 VOP sets a dma_stop bit when it's done scanning out a frame
> > and needs the driver to acknowledge that by clearing the bit.
> >
> > So unless we clear it "between" frames, the RGB output only shows
> > noise
> > instead of the picture. vblank seems to be the most appropriate place
> > to
> > do it, since it indicates exactly that: that the hardware is done
> > with the frame.
> >
> > This seems to be a redundant synchronization mechanism that was
> > removed
> > in later iterations of the VOP hardware block.
> >
> > Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Val Packett <val@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++++++
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
> > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
> > 3 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index a13473b2d..2731fe2b2 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -1766,6 +1766,12 @@ static void vop_handle_vblank(struct vop *vop)
> > }
> > spin_unlock(&drm->event_lock);
> >
> > + if (VOP_HAS_REG(vop, common, dma_stop)) {
> > + spin_lock(&vop->reg_lock);
> > + VOP_REG_SET(vop, common, dma_stop, 0);
> > + spin_unlock(&vop->reg_lock);
> > + }
> > +
>
> Oops… so doing it here actually causes deadlocks, unless we also
> change all other reg_lock usages to be spin_lock_irq/spin_unlock_irq.
>
> Not sure if doing that or going back to v1 would be better.
then please go back to v1 (as v4) :-) .
I.e. regular spinlock vs. spin_lock_irq will have performance
implications and this "feature" is a one-time only thing used
only on a more than 10 year old soc, so it really must not affect
stuff coming after it.
Heiko
