* [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 7:16 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 7:16 UTC (permalink / raw)
Cc: Val Packett, stable, Sandy Huang, Heiko Stübner, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-rockchip,
linux-kernel
On the RK3066, there is a bit that must be cleared, otherwise
the picture does not show up on the display (at least for RGB).
Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: stable@vger.kernel.org
Signed-off-by: Val Packett <val@packett.cool>
---
v2: doing this on vblank makes more sense; added fixes tag
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 7:16 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 7:16 UTC (permalink / raw)
Cc: Val Packett, stable, Sandy Huang, Heiko Stübner, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-rockchip,
linux-kernel
On the RK3066, there is a bit that must be cleared, otherwise
the picture does not show up on the display (at least for RGB).
Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: stable@vger.kernel.org
Signed-off-by: Val Packett <val@packett.cool>
---
v2: doing this on vblank makes more sense; added fixes tag
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
2024-05-27 7:16 ` Val Packett
(?)
@ 2024-05-27 7:16 ` Val Packett
-1 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 7:16 UTC (permalink / raw)
Cc: Val Packett, stable, Sandy Huang, Heiko Stübner, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-rockchip,
linux-kernel
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>
---
v2: expanded commit message
---
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-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
@ 2024-05-27 7:16 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 7:16 UTC (permalink / raw)
Cc: Val Packett, stable, Sandy Huang, Heiko Stübner, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-rockchip,
linux-kernel
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>
---
v2: expanded commit message
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH v2 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
@ 2024-05-27 7:16 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 7:16 UTC (permalink / raw)
Cc: Val Packett, stable, Sandy Huang, Heiko Stübner, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Daniel Vetter, dri-devel, linux-arm-kernel, linux-rockchip,
linux-kernel
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>
---
v2: expanded commit message
---
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
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
2024-05-27 7:16 ` Val Packett
(?)
@ 2024-05-27 20:43 ` Heiko Stübner
-1 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 20:43 UTC (permalink / raw)
To: Val Packett
Cc: Val Packett, stable, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
dri-devel, linux-arm-kernel, linux-rockchip, linux-kernel
Hi Val,
Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> On the RK3066, there is a bit that must be cleared, otherwise
> the picture does not show up on the display (at least for RGB).
>
> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> v2: doing this on vblank makes more sense; added fixes tag
can you give a rationale for this please?
I.e. does this dma-stop bit need to be set on each vblank that happens
to push this frame to the display somehow?
Because at least in theory atomic_flush where this was living in in v1,
might happen at a slower interval?
Thanks
Heiko
> ---
> 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),
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 20:43 ` Heiko Stübner
0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 20:43 UTC (permalink / raw)
To: Val Packett
Cc: Val Packett, stable, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
dri-devel, linux-arm-kernel, linux-rockchip, linux-kernel
Hi Val,
Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> On the RK3066, there is a bit that must be cleared, otherwise
> the picture does not show up on the display (at least for RGB).
>
> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> v2: doing this on vblank makes more sense; added fixes tag
can you give a rationale for this please?
I.e. does this dma-stop bit need to be set on each vblank that happens
to push this frame to the display somehow?
Because at least in theory atomic_flush where this was living in in v1,
might happen at a slower interval?
Thanks
Heiko
> ---
> 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),
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 20:43 ` Heiko Stübner
0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 20:43 UTC (permalink / raw)
To: Val Packett
Cc: Val Packett, stable, Sandy Huang, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
dri-devel, linux-arm-kernel, linux-rockchip, linux-kernel
Hi Val,
Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> On the RK3066, there is a bit that must be cleared, otherwise
> the picture does not show up on the display (at least for RGB).
>
> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Val Packett <val@packett.cool>
> ---
> v2: doing this on vblank makes more sense; added fixes tag
can you give a rationale for this please?
I.e. does this dma-stop bit need to be set on each vblank that happens
to push this frame to the display somehow?
Because at least in theory atomic_flush where this was living in in v1,
might happen at a slower interval?
Thanks
Heiko
> ---
> 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),
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
2024-05-27 20:43 ` Heiko Stübner
(?)
@ 2024-05-27 22:13 ` Val Packett
-1 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 22:13 UTC (permalink / raw)
To: Heiko Stübner
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
<heiko@sntech.de> wrote:
> Hi Val,
>
> Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
>> On the RK3066, there is a bit that must be cleared, otherwise
>> the picture does not show up
>> on the display (at least for RGB).
>>
>> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>> v2: doing this on vblank makes more sense; added fixes tag
>
> can you give a rationale for this please?
>
> I.e. does this dma-stop bit need to be set on each vblank that happens
> to push this frame to the display somehow?
The only things I'm 100% sure about:
- that bit is called dma_stop in the Android kernel's header;
- without ever setting that bit to 1, it was getting set to 1 by the
chip itself, as logging the register on flush was showing a 1 in that
position (it was the only set bit - I guess others aren't readable
after cfg_done?);
- without clearing it "between" frames, the whole screen is always
filled with noise, the picture is not visible.
The rest is at least a bit (ha) speculative:
As I understand from what the name implies, the hardware sets it to
indicate that it has scanned out the frame and is waiting for
acknowledgment (clearing) to be able to scan out the next frame. I
guess it's a redundant synchronization mechanism that was removed in
later iterations of the VOP hardware block.
I've been trying to see if moving where I clear the bit affects the
sort-of-tearing-but-vertical glitches that sometimes happen, especially
early on after the system has just booted, but that seems to be
completely unrelated pixel clock craziness (the Android kernel runs the
screen at 66 fps, interestingly).
I'm fairly confident that both places are "correct". The reason I'm
more on the side of vblank now is that it made logical sense to me when
I thought about it more: acknowledging that the frame has been scanned
out is a reaction to the frame having been scanned out. It's a
consequence of *that* that the acknowledgment is required for the next
frame to be drawn.
Unless we can get the opinion of someone closely familiar with this
decade-old hardware, we only have this reasoning to go off of :)
~val
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 22:13 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 22:13 UTC (permalink / raw)
To: Heiko Stübner
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
<heiko@sntech.de> wrote:
> Hi Val,
>
> Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
>> On the RK3066, there is a bit that must be cleared, otherwise
>> the picture does not show up
>> on the display (at least for RGB).
>>
>> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>> v2: doing this on vblank makes more sense; added fixes tag
>
> can you give a rationale for this please?
>
> I.e. does this dma-stop bit need to be set on each vblank that happens
> to push this frame to the display somehow?
The only things I'm 100% sure about:
- that bit is called dma_stop in the Android kernel's header;
- without ever setting that bit to 1, it was getting set to 1 by the
chip itself, as logging the register on flush was showing a 1 in that
position (it was the only set bit - I guess others aren't readable
after cfg_done?);
- without clearing it "between" frames, the whole screen is always
filled with noise, the picture is not visible.
The rest is at least a bit (ha) speculative:
As I understand from what the name implies, the hardware sets it to
indicate that it has scanned out the frame and is waiting for
acknowledgment (clearing) to be able to scan out the next frame. I
guess it's a redundant synchronization mechanism that was removed in
later iterations of the VOP hardware block.
I've been trying to see if moving where I clear the bit affects the
sort-of-tearing-but-vertical glitches that sometimes happen, especially
early on after the system has just booted, but that seems to be
completely unrelated pixel clock craziness (the Android kernel runs the
screen at 66 fps, interestingly).
I'm fairly confident that both places are "correct". The reason I'm
more on the side of vblank now is that it made logical sense to me when
I thought about it more: acknowledging that the frame has been scanned
out is a reaction to the frame having been scanned out. It's a
consequence of *that* that the acknowledgment is required for the next
frame to be drawn.
Unless we can get the opinion of someone closely familiar with this
decade-old hardware, we only have this reasoning to go off of :)
~val
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 22:13 ` Val Packett
0 siblings, 0 replies; 33+ messages in thread
From: Val Packett @ 2024-05-27 22:13 UTC (permalink / raw)
To: Heiko Stübner
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
<heiko@sntech.de> wrote:
> Hi Val,
>
> Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
>> On the RK3066, there is a bit that must be cleared, otherwise
>> the picture does not show up
>> on the display (at least for RGB).
>>
>> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Val Packett <val@packett.cool>
>> ---
>> v2: doing this on vblank makes more sense; added fixes tag
>
> can you give a rationale for this please?
>
> I.e. does this dma-stop bit need to be set on each vblank that happens
> to push this frame to the display somehow?
The only things I'm 100% sure about:
- that bit is called dma_stop in the Android kernel's header;
- without ever setting that bit to 1, it was getting set to 1 by the
chip itself, as logging the register on flush was showing a 1 in that
position (it was the only set bit - I guess others aren't readable
after cfg_done?);
- without clearing it "between" frames, the whole screen is always
filled with noise, the picture is not visible.
The rest is at least a bit (ha) speculative:
As I understand from what the name implies, the hardware sets it to
indicate that it has scanned out the frame and is waiting for
acknowledgment (clearing) to be able to scan out the next frame. I
guess it's a redundant synchronization mechanism that was removed in
later iterations of the VOP hardware block.
I've been trying to see if moving where I clear the bit affects the
sort-of-tearing-but-vertical glitches that sometimes happen, especially
early on after the system has just booted, but that seems to be
completely unrelated pixel clock craziness (the Android kernel runs the
screen at 66 fps, interestingly).
I'm fairly confident that both places are "correct". The reason I'm
more on the side of vblank now is that it made logical sense to me when
I thought about it more: acknowledging that the frame has been scanned
out is a reaction to the frame having been scanned out. It's a
consequence of *that* that the acknowledgment is required for the next
frame to be drawn.
Unless we can get the opinion of someone closely familiar with this
decade-old hardware, we only have this reasoning to go off of :)
~val
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
2024-05-27 22:13 ` Val Packett
(?)
@ 2024-05-27 22:51 ` Heiko Stübner
-1 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 22:51 UTC (permalink / raw)
To: Val Packett
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
Hey,
Am Dienstag, 28. Mai 2024, 00:13:59 CEST schrieb Val Packett:
> On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
> <heiko@sntech.de> wrote:
> > Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> >> On the RK3066, there is a bit that must be cleared, otherwise
> >> the picture does not show up
> >> on the display (at least for RGB).
> >>
> >> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Val Packett <val@packett.cool>
> >> ---
> >> v2: doing this on vblank makes more sense; added fixes tag
> >
> > can you give a rationale for this please?
> >
> > I.e. does this dma-stop bit need to be set on each vblank that happens
> > to push this frame to the display somehow?
>
>
> The only things I'm 100% sure about:
>
> - that bit is called dma_stop in the Android kernel's header;
> - without ever setting that bit to 1, it was getting set to 1 by the
> chip itself, as logging the register on flush was showing a 1 in that
> position (it was the only set bit - I guess others aren't readable
> after cfg_done?);
> - without clearing it "between" frames, the whole screen is always
> filled with noise, the picture is not visible.
>
> The rest is at least a bit (ha) speculative:
>
> As I understand from what the name implies, the hardware sets it to
> indicate that it has scanned out the frame and is waiting for
> acknowledgment (clearing) to be able to scan out the next frame. I
> guess it's a redundant synchronization mechanism that was removed in
> later iterations of the VOP hardware block.
>
> I've been trying to see if moving where I clear the bit affects the
> sort-of-tearing-but-vertical glitches that sometimes happen, especially
> early on after the system has just booted, but that seems to be
> completely unrelated pixel clock craziness (the Android kernel runs the
> screen at 66 fps, interestingly).
>
> I'm fairly confident that both places are "correct". The reason I'm
> more on the side of vblank now is that it made logical sense to me when
> I thought about it more: acknowledging that the frame has been scanned
> out is a reaction to the frame having been scanned out. It's a
> consequence of *that* that the acknowledgment is required for the next
> frame to be drawn.
>
> Unless we can get the opinion of someone closely familiar with this
> decade-old hardware, we only have this reasoning to go off of :)
Actually that reasoning was exactly what I was hoping for :-) .
And it actually also makes perfect sense.
I was somehow thinking this needs to be set only when starting output
and not as sort of an Ack.
Could you do a v3 with:
- the findings from above slightly condensed in the commit message
It's really helpful when someone stumbles onto that commit 10 years
from now and can get this really helpful explanation from the commit
message.
- sending it as a _new_ thread
Having v2 as reply to v1 patches confuses tooling that then can't
distinguish what is actually part of this v2
Thanks a lot
Heiko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 22:51 ` Heiko Stübner
0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 22:51 UTC (permalink / raw)
To: Val Packett
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
Hey,
Am Dienstag, 28. Mai 2024, 00:13:59 CEST schrieb Val Packett:
> On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
> <heiko@sntech.de> wrote:
> > Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> >> On the RK3066, there is a bit that must be cleared, otherwise
> >> the picture does not show up
> >> on the display (at least for RGB).
> >>
> >> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Val Packett <val@packett.cool>
> >> ---
> >> v2: doing this on vblank makes more sense; added fixes tag
> >
> > can you give a rationale for this please?
> >
> > I.e. does this dma-stop bit need to be set on each vblank that happens
> > to push this frame to the display somehow?
>
>
> The only things I'm 100% sure about:
>
> - that bit is called dma_stop in the Android kernel's header;
> - without ever setting that bit to 1, it was getting set to 1 by the
> chip itself, as logging the register on flush was showing a 1 in that
> position (it was the only set bit - I guess others aren't readable
> after cfg_done?);
> - without clearing it "between" frames, the whole screen is always
> filled with noise, the picture is not visible.
>
> The rest is at least a bit (ha) speculative:
>
> As I understand from what the name implies, the hardware sets it to
> indicate that it has scanned out the frame and is waiting for
> acknowledgment (clearing) to be able to scan out the next frame. I
> guess it's a redundant synchronization mechanism that was removed in
> later iterations of the VOP hardware block.
>
> I've been trying to see if moving where I clear the bit affects the
> sort-of-tearing-but-vertical glitches that sometimes happen, especially
> early on after the system has just booted, but that seems to be
> completely unrelated pixel clock craziness (the Android kernel runs the
> screen at 66 fps, interestingly).
>
> I'm fairly confident that both places are "correct". The reason I'm
> more on the side of vblank now is that it made logical sense to me when
> I thought about it more: acknowledging that the frame has been scanned
> out is a reaction to the frame having been scanned out. It's a
> consequence of *that* that the acknowledgment is required for the next
> frame to be drawn.
>
> Unless we can get the opinion of someone closely familiar with this
> decade-old hardware, we only have this reasoning to go off of :)
Actually that reasoning was exactly what I was hoping for :-) .
And it actually also makes perfect sense.
I was somehow thinking this needs to be set only when starting output
and not as sort of an Ack.
Could you do a v3 with:
- the findings from above slightly condensed in the commit message
It's really helpful when someone stumbles onto that commit 10 years
from now and can get this really helpful explanation from the commit
message.
- sending it as a _new_ thread
Having v2 as reply to v1 patches confuses tooling that then can't
distinguish what is actually part of this v2
Thanks a lot
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
@ 2024-05-27 22:51 ` Heiko Stübner
0 siblings, 0 replies; 33+ messages in thread
From: Heiko Stübner @ 2024-05-27 22:51 UTC (permalink / raw)
To: Val Packett
Cc: stable, Sandy Huang, Andy Yan, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, dri-devel,
linux-arm-kernel, linux-rockchip, linux-kernel
Hey,
Am Dienstag, 28. Mai 2024, 00:13:59 CEST schrieb Val Packett:
> On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
> <heiko@sntech.de> wrote:
> > Am Montag, 27. Mai 2024, 09:16:33 CEST schrieb Val Packett:
> >> On the RK3066, there is a bit that must be cleared, otherwise
> >> the picture does not show up
> >> on the display (at least for RGB).
> >>
> >> Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Val Packett <val@packett.cool>
> >> ---
> >> v2: doing this on vblank makes more sense; added fixes tag
> >
> > can you give a rationale for this please?
> >
> > I.e. does this dma-stop bit need to be set on each vblank that happens
> > to push this frame to the display somehow?
>
>
> The only things I'm 100% sure about:
>
> - that bit is called dma_stop in the Android kernel's header;
> - without ever setting that bit to 1, it was getting set to 1 by the
> chip itself, as logging the register on flush was showing a 1 in that
> position (it was the only set bit - I guess others aren't readable
> after cfg_done?);
> - without clearing it "between" frames, the whole screen is always
> filled with noise, the picture is not visible.
>
> The rest is at least a bit (ha) speculative:
>
> As I understand from what the name implies, the hardware sets it to
> indicate that it has scanned out the frame and is waiting for
> acknowledgment (clearing) to be able to scan out the next frame. I
> guess it's a redundant synchronization mechanism that was removed in
> later iterations of the VOP hardware block.
>
> I've been trying to see if moving where I clear the bit affects the
> sort-of-tearing-but-vertical glitches that sometimes happen, especially
> early on after the system has just booted, but that seems to be
> completely unrelated pixel clock craziness (the Android kernel runs the
> screen at 66 fps, interestingly).
>
> I'm fairly confident that both places are "correct". The reason I'm
> more on the side of vblank now is that it made logical sense to me when
> I thought about it more: acknowledging that the frame has been scanned
> out is a reaction to the frame having been scanned out. It's a
> consequence of *that* that the acknowledgment is required for the next
> frame to be drawn.
>
> Unless we can get the opinion of someone closely familiar with this
> decade-old hardware, we only have this reasoning to go off of :)
Actually that reasoning was exactly what I was hoping for :-) .
And it actually also makes perfect sense.
I was somehow thinking this needs to be set only when starting output
and not as sort of an Ack.
Could you do a v3 with:
- the findings from above slightly condensed in the commit message
It's really helpful when someone stumbles onto that commit 10 years
from now and can get this really helpful explanation from the commit
message.
- sending it as a _new_ thread
Having v2 as reply to v1 patches confuses tooling that then can't
distinguish what is actually part of this v2
Thanks a lot
Heiko
^ permalink raw reply [flat|nested] 33+ messages in thread