[PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

[PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

[Val Packett]

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@vger.kernel.org
Signed-off-by: Val Packett <val@packett.cool>
---
 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);
+	}
+
 	if (test_and_clear_bit(VOP_PENDING_FB_UNREF, &vop->pending))
 		drm_flip_work_commit(&vop->fb_unref_work, system_unbound_wq);
 }
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index b33e5bdc2..0cf512cc1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -122,6 +122,7 @@ struct vop_common {
 	struct vop_reg lut_buffer_index;
 	struct vop_reg gate_en;
 	struct vop_reg mmu_en;
+	struct vop_reg dma_stop;
 	struct vop_reg out_mode;
 	struct vop_reg standby;
 };
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index b9ee02061..9bcb40a64 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -466,6 +466,7 @@ static const struct vop_output rk3066_output = {
 };
 
 static const struct vop_common rk3066_common = {
+	.dma_stop = VOP_REG(RK3066_SYS_CTRL0, 0x1, 0),
 	.standby = VOP_REG(RK3066_SYS_CTRL0, 0x1, 1),
 	.out_mode = VOP_REG(RK3066_DSP_CTRL0, 0xf, 0),
 	.cfg_done = VOP_REG(RK3066_REG_CFG_DONE, 0x1, 0),
-- 
2.45.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066

[Val Packett]

The RK3066 does have RGB display output, so it should be marked as such.

Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: stable@vger.kernel.org
Signed-off-by: Val Packett <val@packett.cool>
---
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 9bcb40a64..e2c6ba26f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -515,6 +515,7 @@ static const struct vop_data rk3066_vop = {
 	.output = &rk3066_output,
 	.win = rk3066_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
+	.feature = VOP_FEATURE_INTERNAL_RGB,
 	.max_output = { 1920, 1080 },
 };
 
-- 
2.45.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

[Val Packett]

On Mon, May 27 2024 at 20:11:49 -03:00:00, Val Packett 
<val@packett.cool> 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@vger.kernel.org
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  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.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066

[Heiko Stuebner]

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@packett.cool> 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@vger.kernel.org
> > Signed-off-by: Val Packett <val@packett.cool>
> > ---
> >  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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel