[PATCH v4 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
The RK3066 does have RGB display output, so it should be marked as such.
Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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 = _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.2
[PATCH v4 1/2] drm/rockchip: vop: clear DMA stop bit on RK3066
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.
Unless we clear it "between" frames, the RGB output only shows noise
instead of the picture. atomic_flush is the place for it that least
affects other code (doing it on vblank would require converting all
other usages of the reg_lock to spin_(un)lock_irq, which would affect
performance for everyone).
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: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
3 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a13473b2d..e88fbd568 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1583,6 +1583,10 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
VOP_AFBC_SET(vop, enable, s->enable_afbc);
vop_cfg_done(vop);
+ /* Ack the DMA transfer of the previous frame (RK3066). */
+ if (VOP_HAS_REG(vop, common, dma_stop))
+ VOP_REG_SET(vop, common, dma_stop, 0);
+
spin_unlock(>reg_lock);
/*
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.2
[PATCH v4 0/2] drm/rockchip: vop: Support internal display on RK3066
This tiny patch series makes the changes required to enable the RGB display output on the Rockchip RK3066 SoC. Final version as discussed. Val Packett (2): drm/rockchip: vop: clear DMA stop bit on RK3066 drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 2 ++ 3 files changed, 7 insertions(+) -- 2.45.2
Re: [PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
On Mon, May 27 2024 at 20:11:49 -03:00:00, Val Packett
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: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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(>event_lock);
+ if (VOP_HAS_REG(vop, common, dma_stop)) {
+ spin_lock(>reg_lock);
+ VOP_REG_SET(vop, common, dma_stop, 0);
+ spin_unlock(>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.
[PATCH v3 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
The RK3066 does have RGB display output, so it should be marked as such.
Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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 = _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
[PATCH v3 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
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: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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(>event_lock);
+ if (VOP_HAS_REG(vop, common, dma_stop)) {
+ spin_lock(>reg_lock);
+ VOP_REG_SET(vop, common, dma_stop, 0);
+ spin_unlock(>reg_lock);
+ }
+
if (test_and_clear_bit(VOP_PENDING_FB_UNREF, >pending))
drm_flip_work_commit(>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
Re: [PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
On Mon, May 27 2024 at 22:43:18 +02:00:00, Heiko Stübner
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: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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
[PATCH v2 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
The RK3066 does have RGB display output, so it should be marked as such.
Fixes: f4a6de8 ("drm: rockchip: vop: add rk3066 vop definitions")
Cc: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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 = _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
[PATCH v2 1/2] drm/rockchip: vop: clear DMA stop bit upon vblank on RK3066
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: sta...@vger.kernel.org
Signed-off-by: Val Packett
---
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(>event_lock);
+ if (VOP_HAS_REG(vop, common, dma_stop)) {
+ spin_lock(>reg_lock);
+ VOP_REG_SET(vop, common, dma_stop, 0);
+ spin_unlock(>reg_lock);
+ }
+
if (test_and_clear_bit(VOP_PENDING_FB_UNREF, >pending))
drm_flip_work_commit(>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
Re: [PATCH v4 2/3] drm/lima: Fix dma_resv deadlock at drm object pin time
On Thu, May 23 2024 at 12:32:18 +01:00:00, Adrián Larumbe
wrote:
Commit a78027847226 ("drm/gem: Acquire reservation lock in
drm_gem_{pin/unpin}()") moved locking the DRM object's dma
reservation to
drm_gem_pin(), but Lima's pin callback kept calling drm_gem_shmem_pin,
which also tries to lock the same dma_resv, leading to a double lock
situation.
As was already done for Panfrost in the previous commit, fix it by
replacing drm_gem_shmem_pin() with its locked variant.
Hi, just found this while dealing with compositor lockups upon
launching a GL client on an old Rockchip RK3066 tablet, and it did fix
the problem :) Thank you.
Tested-by: Val Packett
Re: [PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On Sun, May 19 2024 at 09:59:47 +02:00:00, Greg KH wrote: On Sun, May 19, 2024 at 04:31:31AM -0300, Val Packett wrote: On the RK3066, there is a bit that must be cleared on flush, otherwise we do not get display output (at least for RGB). What commit id does this fix? I guess: f4a6de855e "drm: rockchip: vop: add rk3066 vop definitions" ? But similar changes like: 742203cd "drm: rockchip: add missing registers for RK3066" 8d544233 "drm/rockchip: vop: Add directly output rgb feature for px30" did not have any "Fixes" reference. ~val
[PATCH 1/2] drm/rockchip: vop: clear DMA stop bit on flush on RK3066
On the RK3066, there is a bit that must be cleared on flush, otherwise
we do not get display output (at least for RGB).
Signed-off-by: Val Packett
Cc: sta...@vger.kernel.org
---
Hi! This was required to get display working on an old RK3066 tablet,
along with the next tiny patch in the series enabling the RGB output.
I have spent quite a lot of time banging my head against the wall debugging
that display (especially since at the same time a scaler chip is used for
LVDS encoding), but finally adding debug prints showed that RK3066_SYS_CTRL0
ended up being reset to all-zero after being written correctly upon init.
Looking at the register definitions in the vendor driver revealed that the
reason was pretty self-explanatory: "dma_stop".
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +++
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 +
drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a13473b2d..d4daeba74 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1578,6 +1578,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock(>reg_lock);
+ /* If the chip has a DMA stop bit (RK3066), it must be cleared. */
+ VOP_REG_SET(vop, common, dma_stop, 0);
+
/* Enable AFBC if there is some AFBC window, disable otherwise. */
s = to_rockchip_crtc_state(crtc->state);
VOP_AFBC_SET(vop, enable, s->enable_afbc);
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.0
[PATCH 2/2] drm/rockchip: vop: enable VOP_FEATURE_INTERNAL_RGB on RK3066
Signed-off-by: Val Packett
Cc: sta...@vger.kernel.org
---
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 = _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.0
