diff mbox series

[v2] drm/rockchip: vop: Support dithering to RGB666

Message ID 20190219100848.2222-1-urjaman@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/rockchip: vop: Support dithering to RGB666 | expand

Commit Message

Urja Rannikko Feb. 19, 2019, 10:08 a.m. UTC 
Splits out the dither register bits and introduces
the same config enumerations as in the rockchip kernel tree.
Tested to fix the banding on my ASUS C201.

Signed-off-by: Urja Rannikko <urjaman@gmail.com>
---
This is a version of the patch that is quite inspired by the rockchip
kernel tree (that i hadn't looked at enough), but still keeps
to just implementing RGB666 dithering because more changes
are needed to get the information in place for detecting RGB565,
but adding that should be easier afterwards.

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 25 +++++++++++++++++----
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 14 +++++++++++-
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 18 +++++++++++++--
 3 files changed, 50 insertions(+), 7 deletions(-)

Comments

Urja Rannikko March 13, 2019, 8:26 p.m. UTC | #1 
Hi,

Umm, Ping?
Johan Jonker March 14, 2019, 5:20 p.m. UTC | #2 
How about RK3066? See/use linux-next.
Urja Rannikko March 18, 2019, 1:47 p.m. UTC | #3 
On Thu, Mar 14, 2019 at 5:20 PM Johan Jonker <jbx6244@gmail.com> wrote:
>
> How about RK3066? See/use linux-next.
Hi and thanks for the notice. The rest of this mail "addressed" for
anyone interested.

I've added the dither bits for RK3066 - it doesnt have the bit for
Allegro/FRC (sel)
which really isnt a problem, but brought up a question for me:
Should the code avoid calling vop_reg_write with unsupported registers/bits?
I assumed a yes, but based on my tests it already does it on RK3288..
(atleast x/y_mir_en and act_info iirc).

This only results in a "Warning: not support reg_name" print in drm
debug output,
but to me printing warnings during normal operations (even if they're
only in debug output)
seems wrong.

I'll be sending a v3 of the patch after this mail, and i did avoid the
unnecessary
write for now, but i expect to be changing that line of code :P
Heiko Stübner March 18, 2019, 2:01 p.m. UTC | #4 
Hi Urja,

Am Montag, 18. März 2019, 14:47:37 CET schrieb Urja Rannikko:
> On Thu, Mar 14, 2019 at 5:20 PM Johan Jonker <jbx6244@gmail.com> wrote:
> > How about RK3066? See/use linux-next.
> 
> Hi and thanks for the notice. The rest of this mail "addressed" for
> anyone interested.

would be me I guess ;-) .

And I was also just looking at the v2 today.

DRM tends to be difficult, as I'm not _that_ confident in spotting all
things while just looking at the patch, that I want to do a roundtrip of
testing on the Rockchip boards I have in my farm.

And finding that time is surprisingly difficult, especially as I haven't
yet managed to export graphical output - in contrast I can do all non-
graphic testing from everywhere with an internet connection.

So in any case, sorry about letting this sit for waaaay to long.


> I've added the dither bits for RK3066 - it doesnt have the bit for
> Allegro/FRC (sel)
> which really isnt a problem, but brought up a question for me:
> Should the code avoid calling vop_reg_write with unsupported registers/bits?
> I assumed a yes, but based on my tests it already does it on RK3288..
> (atleast x/y_mir_en and act_info iirc).
> 
> This only results in a "Warning: not support reg_name" print in drm
> debug output,
> but to me printing warnings during normal operations (even if they're
> only in debug output) seems wrong.

Rockchip VOPs have the issue, that it seems soc designers make it a game
to move as much registers around as possible between each implementation.

So I guess the silently ignoring of non-existent registers was somehow the
easiest way of dealing with that gracefully.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 0c35a88e33dd..98c8b5b7627b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -951,10 +951,27 @@  static void vop_crtc_atomic_enable(struct drm_crtc *crtc,
 	    !(vop_data->feature & VOP_FEATURE_OUTPUT_RGB10))
 		s->output_mode = ROCKCHIP_OUT_MODE_P888;
 
-	if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && s->output_bpc == 8)
-		VOP_REG_SET(vop, common, pre_dither_down, 1);
-	else
-		VOP_REG_SET(vop, common, pre_dither_down, 0);
+	if (s->output_bpc) { /* Only dither if bpc known. */
+		if (vop_data->feature & VOP_FEATURE_OUTPUT_RGB10) {
+			if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && s->output_bpc <= 8)
+				VOP_REG_SET(vop, common, pre_dither_down, 1);
+			else
+				VOP_REG_SET(vop, common, pre_dither_down, 0);
+		}
+		if (s->output_bpc == 6) {
+			/* Enable allegro dither to RGB666 */
+			VOP_REG_SET(vop, common, dither_down_sel, DITHER_DOWN_ALLEGRO);
+			VOP_REG_SET(vop, common, dither_down_mode, RGB888_TO_RGB666);
+			VOP_REG_SET(vop, common, dither_down_en, 1);
+		} else {
+			VOP_REG_SET(vop, common, dither_down_en, 0);
+		}
+	} else {
+		if (vop_data->feature & VOP_FEATURE_OUTPUT_RGB10)
+			VOP_REG_SET(vop, common, pre_dither_down, 0);
+
+		VOP_REG_SET(vop, common, dither_down_en, 0);
+	}
 
 	VOP_REG_SET(vop, common, out_mode, s->output_mode);
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index fd5765dfd637..92050de140b6 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -68,7 +68,9 @@  struct vop_common {
 	struct vop_reg dsp_blank;
 	struct vop_reg data_blank;
 	struct vop_reg pre_dither_down;
-	struct vop_reg dither_down;
+	struct vop_reg dither_down_sel;
+	struct vop_reg dither_down_mode;
+	struct vop_reg dither_down_en;
 	struct vop_reg dither_up;
 	struct vop_reg gate_en;
 	struct vop_reg mmu_en;
@@ -268,6 +270,16 @@  enum scale_down_mode {
 	SCALE_DOWN_AVG = 0x1
 };
 
+enum dither_down_mode {
+	RGB888_TO_RGB565 = 0x0,
+	RGB888_TO_RGB666 = 0x1
+};
+
+enum dither_down_mode_sel {
+	DITHER_DOWN_ALLEGRO = 0x0,
+	DITHER_DOWN_FRC = 0x1
+};
+
 enum vop_pol {
 	HSYNC_POSITIVE = 0,
 	VSYNC_POSITIVE = 1,
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index a6db3cd5544b..59266c473795 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -137,6 +137,9 @@  static const struct vop_common rk3036_common = {
 	.standby = VOP_REG_SYNC(RK3036_SYS_CTRL, 0x1, 30),
 	.out_mode = VOP_REG(RK3036_DSP_CTRL0, 0xf, 0),
 	.dsp_blank = VOP_REG(RK3036_DSP_CTRL1, 0x1, 24),
+	.dither_down_sel = VOP_REG(RK3036_DSP_CTRL0, 0x1, 27),
+	.dither_down_en = VOP_REG(RK3036_DSP_CTRL0, 0x1, 11),
+	.dither_down_mode = VOP_REG(RK3036_DSP_CTRL0, 0x1, 10),
 	.cfg_done = VOP_REG_SYNC(RK3036_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -200,6 +203,9 @@  static const struct vop_common px30_common = {
 	.standby = VOP_REG_SYNC(PX30_SYS_CTRL2, 0x1, 1),
 	.out_mode = VOP_REG(PX30_DSP_CTRL2, 0xf, 16),
 	.dsp_blank = VOP_REG(PX30_DSP_CTRL2, 0x1, 14),
+	.dither_down_en = VOP_REG(PX30_DSP_CTRL2, 0x1, 8),
+	.dither_down_sel = VOP_REG(PX30_DSP_CTRL2, 0x1, 7),
+	.dither_down_mode = VOP_REG(PX30_DSP_CTRL2, 0x1, 6),
 	.cfg_done = VOP_REG_SYNC(PX30_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -350,6 +356,9 @@  static const struct vop_common rk3188_common = {
 	.standby = VOP_REG(RK3188_SYS_CTRL, 0x1, 30),
 	.out_mode = VOP_REG(RK3188_DSP_CTRL0, 0xf, 0),
 	.cfg_done = VOP_REG(RK3188_REG_CFG_DONE, 0x1, 0),
+	.dither_down_sel = VOP_REG(RK3188_DSP_CTRL0, 0x1, 27),
+	.dither_down_en = VOP_REG(RK3188_DSP_CTRL0, 0x1, 11),
+	.dither_down_mode = VOP_REG(RK3188_DSP_CTRL0, 0x1, 10),
 	.dsp_blank = VOP_REG(RK3188_DSP_CTRL1, 0x3, 24),
 };
 
@@ -473,8 +482,10 @@  static const struct vop_common rk3288_common = {
 	.standby = VOP_REG_SYNC(RK3288_SYS_CTRL, 0x1, 22),
 	.gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23),
 	.mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20),
+	.dither_down_sel = VOP_REG(RK3288_DSP_CTRL1, 0x1, 4),
+	.dither_down_mode = VOP_REG(RK3288_DSP_CTRL1, 0x1, 3),
+	.dither_down_en = VOP_REG(RK3288_DSP_CTRL1, 0x1, 2),
 	.pre_dither_down = VOP_REG(RK3288_DSP_CTRL1, 0x1, 1),
-	.dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1),
 	.dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6),
 	.data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
 	.dsp_blank = VOP_REG(RK3288_DSP_CTRL0, 0x3, 18),
@@ -707,7 +718,10 @@  static const struct vop_misc rk3328_misc = {
 
 static const struct vop_common rk3328_common = {
 	.standby = VOP_REG_SYNC(RK3328_SYS_CTRL, 0x1, 22),
-	.dither_down = VOP_REG(RK3328_DSP_CTRL1, 0xf, 1),
+	.dither_down_sel = VOP_REG(RK3328_DSP_CTRL1, 0x1, 4),
+	.dither_down_mode = VOP_REG(RK3328_DSP_CTRL1, 0x1, 3),
+	.dither_down_en = VOP_REG(RK3328_DSP_CTRL1, 0x1, 2),
+	.pre_dither_down = VOP_REG(RK3328_DSP_CTRL1, 0x1, 1),
 	.dither_up = VOP_REG(RK3328_DSP_CTRL1, 0x1, 6),
 	.dsp_blank = VOP_REG(RK3328_DSP_CTRL0, 0x3, 18),
 	.out_mode = VOP_REG(RK3328_DSP_CTRL0, 0xf, 0),