[PATCH v4 0/4] drm/rockchip: dw_hdmi: Add 4k@30 support

[PATCH v4 0/4] drm/rockchip: dw_hdmi: Add 4k@30 support

[Sascha Hauer]

Another round of these patches. Only change this time is the addition
of a patch to limit the resolutions to the ones supported by the VOP
driver which should resolve the regression reported by FUKAUMI Naoki.

Sascha

Changes since v3:
- Add patch to limit VOP resolutions to hardware capabilities

Changes since v2:
- Use correct register values for mpll_cfg
- Add patch to discard modes we cannot achieve

Changes since v1:
- Allow non standard clock rates only on Synopsys phy as suggested by
  Robin Murphy

Sascha Hauer (4):
  drm/rockchip: vop: limit maximium resolution to hardware capabilities
  drm/rockchip: dw_hdmi: relax mode_valid hook
  drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
  drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 40 ++++++++++++++++-----
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 ++++++++++++++++
 4 files changed, 80 insertions(+), 8 deletions(-)

-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 0/4] drm/rockchip: dw_hdmi: Add 4k@30 support

[Sascha Hauer]

Another round of these patches. Only change this time is the addition
of a patch to limit the resolutions to the ones supported by the VOP
driver which should resolve the regression reported by FUKAUMI Naoki.

Sascha

Changes since v3:
- Add patch to limit VOP resolutions to hardware capabilities

Changes since v2:
- Use correct register values for mpll_cfg
- Add patch to discard modes we cannot achieve

Changes since v1:
- Allow non standard clock rates only on Synopsys phy as suggested by
  Robin Murphy

Sascha Hauer (4):
  drm/rockchip: vop: limit maximium resolution to hardware capabilities
  drm/rockchip: dw_hdmi: relax mode_valid hook
  drm/rockchip: dw_hdmi: Add support for 4k@30 resolution
  drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 40 ++++++++++++++++-----
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 ++++++++++++++++
 4 files changed, 80 insertions(+), 8 deletions(-)

-- 
2.30.2

[PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

The different VOP variants support different maximum resolutions. Reject
resolutions that are not supported by a specific variant.

This hasn't been a problem in the upstream driver so far as 1920x1080
has been the maximum resolution supported by the HDMI driver and that
resolution is supported by all VOP variants. Now with higher resolutions
supported in the HDMI driver we have to limit the resolutions to the
ones supported by the VOP.

The actual maximum resolutions are taken from the Rockchip downstream
Kernel.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---

Notes:
    Changes since v3:
    - new patch

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa1f4ee6d1950..96b6bd8d17803 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&vop->irq_lock, flags);
 }
 
+static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
+						const struct drm_display_mode *mode)
+{
+	struct vop *vop = to_vop(crtc);
+
+	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
+		return MODE_BAD_HVALUE;
+
+	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
+		return MODE_BAD_VVALUE;
+
+	return MODE_OK;
+}
+
 static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
+	.mode_valid = vop_crtc_mode_valid,
 	.mode_fixup = vop_crtc_mode_fixup,
 	.atomic_check = vop_crtc_atomic_check,
 	.atomic_begin = vop_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 8502849833d93..5c4875ca3f270 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -225,6 +225,8 @@ struct vop_data {
 	const struct vop_win_data *win;
 	unsigned int win_size;
 	unsigned int lut_size;
+	unsigned int max_xres;
+	unsigned int max_yres;
 
 #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
 #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 014f99e8928e3..dc1a703d9d1a8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
 	.output = &rk3036_output,
 	.win = rk3036_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_win_phy rk3126_win1_data = {
@@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
 	.output = &rk3036_output,
 	.win = rk3126_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const int px30_vop_intrs[] = {
@@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
 	.output = &px30_output,
 	.win = px30_vop_big_win_data,
 	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_win_data px30_vop_lit_win_data[] = {
@@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
 	.output = &px30_output,
 	.win = px30_vop_lit_win_data,
 	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_scl_regs rk3066_win_scl = {
@@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
 	.output = &rk3066_output,
 	.win = rk3066_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_scl_regs rk3188_win_scl = {
@@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
 	.win = rk3188_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
 	.feature = VOP_FEATURE_INTERNAL_RGB,
+	.max_xres = 2048,
+	.max_yres = 1536,
 };
 
 static const struct vop_scl_extension rk3288_win_full_scl_ext = {
@@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
 	.win = rk3288_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
 	.lut_size = 1024,
+	/*
+	 * This is the maximum resolution for the VOPB, the VOPL can only do
+	 * 2560x1600, but we can't distinguish them as they have the same
+	 * compatible.
+	 */
+	.max_xres = 3840,
+	.max_yres = 2160,
 };
 
 static const int rk3368_vop_intrs[] = {
@@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3368_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_intr rk3366_vop_intr = {
@@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3368_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_output rk3399_output = {
@@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
 	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
 	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
 	.lut_size = 1024,
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_win_data rk3399_vop_lit_win_data[] = {
@@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
 	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
 	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
 	.lut_size = 256,
+	.max_xres = 2560,
+	.max_yres = 1600,
 };
 
 static const struct vop_win_data rk3228_vop_win_data[] = {
@@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3228_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_modeset rk3328_modeset = {
@@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
 	.misc = &rk3328_misc,
 	.win = rk3328_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct of_device_id vop_driver_dt_match[] = {
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

The different VOP variants support different maximum resolutions. Reject
resolutions that are not supported by a specific variant.

This hasn't been a problem in the upstream driver so far as 1920x1080
has been the maximum resolution supported by the HDMI driver and that
resolution is supported by all VOP variants. Now with higher resolutions
supported in the HDMI driver we have to limit the resolutions to the
ones supported by the VOP.

The actual maximum resolutions are taken from the Rockchip downstream
Kernel.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---

Notes:
    Changes since v3:
    - new patch

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
 3 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index fa1f4ee6d1950..96b6bd8d17803 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&vop->irq_lock, flags);
 }
 
+static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
+						const struct drm_display_mode *mode)
+{
+	struct vop *vop = to_vop(crtc);
+
+	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
+		return MODE_BAD_HVALUE;
+
+	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
+		return MODE_BAD_VVALUE;
+
+	return MODE_OK;
+}
+
 static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
+	.mode_valid = vop_crtc_mode_valid,
 	.mode_fixup = vop_crtc_mode_fixup,
 	.atomic_check = vop_crtc_atomic_check,
 	.atomic_begin = vop_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index 8502849833d93..5c4875ca3f270 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -225,6 +225,8 @@ struct vop_data {
 	const struct vop_win_data *win;
 	unsigned int win_size;
 	unsigned int lut_size;
+	unsigned int max_xres;
+	unsigned int max_yres;
 
 #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
 #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 014f99e8928e3..dc1a703d9d1a8 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
 	.output = &rk3036_output,
 	.win = rk3036_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_win_phy rk3126_win1_data = {
@@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
 	.output = &rk3036_output,
 	.win = rk3126_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const int px30_vop_intrs[] = {
@@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
 	.output = &px30_output,
 	.win = px30_vop_big_win_data,
 	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_win_data px30_vop_lit_win_data[] = {
@@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
 	.output = &px30_output,
 	.win = px30_vop_lit_win_data,
 	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_scl_regs rk3066_win_scl = {
@@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
 	.output = &rk3066_output,
 	.win = rk3066_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
+	.max_xres = 1920,
+	.max_yres = 1080,
 };
 
 static const struct vop_scl_regs rk3188_win_scl = {
@@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
 	.win = rk3188_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
 	.feature = VOP_FEATURE_INTERNAL_RGB,
+	.max_xres = 2048,
+	.max_yres = 1536,
 };
 
 static const struct vop_scl_extension rk3288_win_full_scl_ext = {
@@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
 	.win = rk3288_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
 	.lut_size = 1024,
+	/*
+	 * This is the maximum resolution for the VOPB, the VOPL can only do
+	 * 2560x1600, but we can't distinguish them as they have the same
+	 * compatible.
+	 */
+	.max_xres = 3840,
+	.max_yres = 2160,
 };
 
 static const int rk3368_vop_intrs[] = {
@@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3368_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_intr rk3366_vop_intr = {
@@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3368_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_output rk3399_output = {
@@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
 	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
 	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
 	.lut_size = 1024,
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_win_data rk3399_vop_lit_win_data[] = {
@@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
 	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
 	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
 	.lut_size = 256,
+	.max_xres = 2560,
+	.max_yres = 1600,
 };
 
 static const struct vop_win_data rk3228_vop_win_data[] = {
@@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
 	.misc = &rk3368_misc,
 	.win = rk3228_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct vop_modeset rk3328_modeset = {
@@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
 	.misc = &rk3328_misc,
 	.win = rk3328_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
+	.max_xres = 4096,
+	.max_yres = 2160,
 };
 
 static const struct of_device_id vop_driver_dt_match[] = {
-- 
2.30.2

[PATCH v4 2/4] drm/rockchip: dw_hdmi: relax mode_valid hook

[Sascha Hauer]

The driver checks if the pixel clock of the given mode matches an entry
in the mpll config table. At least for the Synopsys phy the frequencies
in the mpll table are meant as a frequency range up to which the entry
works, not as a frequency that must match the pixel clock. Return
MODE_OK when the pixelclock is smaller than one of the mpll frequencies
to allow for more display resolutions.
Limit this behaviour to the Synopsys phy at the moment and keep the
current behaviour of forcing exact pixelclock rates for the other phys
until it has been sorted out how and if the vendor specific phys work
with non standard clock rates.

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Link: https://lore.kernel.org/r/20220926080435.259617-2-s.hauer@pengutronix.de
Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-2-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 26 +++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 2f4b8f64cbad3..7d8bf292fedce 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -74,6 +74,7 @@ struct rockchip_hdmi {
 	struct regmap *regmap;
 	struct rockchip_encoder encoder;
 	const struct rockchip_hdmi_chip_data *chip_data;
+	const struct dw_hdmi_plat_data *plat_data;
 	struct clk *ref_clk;
 	struct clk *grf_clk;
 	struct dw_hdmi *hdmi;
@@ -241,23 +242,32 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 }
 
 static enum drm_mode_status
-dw_hdmi_rockchip_mode_valid(struct dw_hdmi *hdmi, void *data,
+dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
 			    const struct drm_display_info *info,
 			    const struct drm_display_mode *mode)
 {
+	struct rockchip_hdmi *hdmi = data;
 	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
 	int pclk = mode->clock * 1000;
-	bool valid = false;
+	bool exact_match = hdmi->plat_data->phy_force_vendor;
 	int i;
 
 	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
-		if (pclk == mpll_cfg[i].mpixelclock) {
-			valid = true;
-			break;
-		}
+		/*
+		 * For vendor specific phys force an exact match of the pixelclock
+		 * to preserve the original behaviour of the driver.
+		 */
+		if (exact_match && pclk == mpll_cfg[i].mpixelclock)
+			return MODE_OK;
+		/*
+		 * The Synopsys phy can work with pixelclocks up to the value given
+		 * in the corresponding mpll_cfg entry.
+		 */
+		if (!exact_match && pclk <= mpll_cfg[i].mpixelclock)
+			return MODE_OK;
 	}
 
-	return (valid) ? MODE_OK : MODE_BAD;
+	return MODE_BAD;
 }
 
 static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder)
@@ -546,8 +556,10 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 		return -ENOMEM;
 
 	hdmi->dev = &pdev->dev;
+	hdmi->plat_data = plat_data;
 	hdmi->chip_data = plat_data->phy_data;
 	plat_data->phy_data = hdmi;
+	plat_data->priv_data = hdmi;
 	encoder = &hdmi->encoder.encoder;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 2/4] drm/rockchip: dw_hdmi: relax mode_valid hook

[Sascha Hauer]

The driver checks if the pixel clock of the given mode matches an entry
in the mpll config table. At least for the Synopsys phy the frequencies
in the mpll table are meant as a frequency range up to which the entry
works, not as a frequency that must match the pixel clock. Return
MODE_OK when the pixelclock is smaller than one of the mpll frequencies
to allow for more display resolutions.
Limit this behaviour to the Synopsys phy at the moment and keep the
current behaviour of forcing exact pixelclock rates for the other phys
until it has been sorted out how and if the vendor specific phys work
with non standard clock rates.

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Link: https://lore.kernel.org/r/20220926080435.259617-2-s.hauer@pengutronix.de
Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-2-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 26 +++++++++++++++------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 2f4b8f64cbad3..7d8bf292fedce 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -74,6 +74,7 @@ struct rockchip_hdmi {
 	struct regmap *regmap;
 	struct rockchip_encoder encoder;
 	const struct rockchip_hdmi_chip_data *chip_data;
+	const struct dw_hdmi_plat_data *plat_data;
 	struct clk *ref_clk;
 	struct clk *grf_clk;
 	struct dw_hdmi *hdmi;
@@ -241,23 +242,32 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
 }
 
 static enum drm_mode_status
-dw_hdmi_rockchip_mode_valid(struct dw_hdmi *hdmi, void *data,
+dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
 			    const struct drm_display_info *info,
 			    const struct drm_display_mode *mode)
 {
+	struct rockchip_hdmi *hdmi = data;
 	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
 	int pclk = mode->clock * 1000;
-	bool valid = false;
+	bool exact_match = hdmi->plat_data->phy_force_vendor;
 	int i;
 
 	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
-		if (pclk == mpll_cfg[i].mpixelclock) {
-			valid = true;
-			break;
-		}
+		/*
+		 * For vendor specific phys force an exact match of the pixelclock
+		 * to preserve the original behaviour of the driver.
+		 */
+		if (exact_match && pclk == mpll_cfg[i].mpixelclock)
+			return MODE_OK;
+		/*
+		 * The Synopsys phy can work with pixelclocks up to the value given
+		 * in the corresponding mpll_cfg entry.
+		 */
+		if (!exact_match && pclk <= mpll_cfg[i].mpixelclock)
+			return MODE_OK;
 	}
 
-	return (valid) ? MODE_OK : MODE_BAD;
+	return MODE_BAD;
 }
 
 static void dw_hdmi_rockchip_encoder_disable(struct drm_encoder *encoder)
@@ -546,8 +556,10 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
 		return -ENOMEM;
 
 	hdmi->dev = &pdev->dev;
+	hdmi->plat_data = plat_data;
 	hdmi->chip_data = plat_data->phy_data;
 	plat_data->phy_data = hdmi;
+	plat_data->priv_data = hdmi;
 	encoder = &hdmi->encoder.encoder;
 
 	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
-- 
2.30.2

[PATCH v4 3/4] drm/rockchip: dw_hdmi: Add support for 4k@30 resolution

[Sascha Hauer]

This adds the PLL/phy settings to support higher resolutions like 4k@30.
The values were taken from the Rockchip downstream Kernel.

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Link: https://lore.kernel.org/r/20220926080435.259617-3-s.hauer@pengutronix.de
Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-3-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 7d8bf292fedce..feba6b9becd6c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -161,6 +161,12 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 			{ 0x214c, 0x0003},
 			{ 0x4064, 0x0003}
 		},
+	}, {
+		340000000, {
+			{ 0x0040, 0x0003 },
+			{ 0x3b4c, 0x0003 },
+			{ 0x5a64, 0x0003 },
+		},
 	}, {
 		~0UL, {
 			{ 0x00a0, 0x000a },
@@ -186,6 +192,8 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 		146250000, { 0x0038, 0x0038, 0x0038 },
 	}, {
 		148500000, { 0x0000, 0x0038, 0x0038 },
+	}, {
+		600000000, { 0x0000, 0x0000, 0x0000 },
 	}, {
 		~0UL,      { 0x0000, 0x0000, 0x0000},
 	}
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 3/4] drm/rockchip: dw_hdmi: Add support for 4k@30 resolution

[Sascha Hauer]

This adds the PLL/phy settings to support higher resolutions like 4k@30.
The values were taken from the Rockchip downstream Kernel.

Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Link: https://lore.kernel.org/r/20220926080435.259617-3-s.hauer@pengutronix.de
Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-3-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 7d8bf292fedce..feba6b9becd6c 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -161,6 +161,12 @@ static const struct dw_hdmi_mpll_config rockchip_mpll_cfg[] = {
 			{ 0x214c, 0x0003},
 			{ 0x4064, 0x0003}
 		},
+	}, {
+		340000000, {
+			{ 0x0040, 0x0003 },
+			{ 0x3b4c, 0x0003 },
+			{ 0x5a64, 0x0003 },
+		},
 	}, {
 		~0UL, {
 			{ 0x00a0, 0x000a },
@@ -186,6 +192,8 @@ static const struct dw_hdmi_curr_ctrl rockchip_cur_ctr[] = {
 		146250000, { 0x0038, 0x0038, 0x0038 },
 	}, {
 		148500000, { 0x0000, 0x0038, 0x0038 },
+	}, {
+		600000000, { 0x0000, 0x0000, 0x0000 },
 	}, {
 		~0UL,      { 0x0000, 0x0000, 0x0000},
 	}
-- 
2.30.2

[PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Sascha Hauer]

The Rockchip PLL drivers are currently table based and support only
the most common pixelclocks. Discard all modes we cannot achieve
at all. Normally the desired pixelclocks have an exact match in the
PLL driver, nevertheless allow for a 0.1% error just in case.

Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index feba6b9becd6c..725952811752b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
 {
 	struct rockchip_hdmi *hdmi = data;
 	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
-	int pclk = mode->clock * 1000;
+	int rpclk, pclk = mode->clock * 1000;
 	bool exact_match = hdmi->plat_data->phy_force_vendor;
 	int i;
 
+	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
+	if (abs(rpclk - pclk) > pclk / 1000)
+		return MODE_NOCLOCK;
+
 	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
 		/*
 		 * For vendor specific phys force an exact match of the pixelclock
-- 
2.30.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Sascha Hauer]

The Rockchip PLL drivers are currently table based and support only
the most common pixelclocks. Discard all modes we cannot achieve
at all. Normally the desired pixelclocks have an exact match in the
PLL driver, nevertheless allow for a 0.1% error just in case.

Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
Tested-by: Dan Johansen <strit@manjaro.org>
Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index feba6b9becd6c..725952811752b 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
 {
 	struct rockchip_hdmi *hdmi = data;
 	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
-	int pclk = mode->clock * 1000;
+	int rpclk, pclk = mode->clock * 1000;
 	bool exact_match = hdmi->plat_data->phy_force_vendor;
 	int i;
 
+	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
+	if (abs(rpclk - pclk) > pclk / 1000)
+		return MODE_NOCLOCK;
+
 	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
 		/*
 		 * For vendor specific phys force an exact match of the pixelclock
-- 
2.30.2

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Dan Johansen]

Den 07.02.2023 kl. 09.44 skrev Sascha Hauer:
> The different VOP variants support different maximum resolutions. Reject
> resolutions that are not supported by a specific variant.
>
> This hasn't been a problem in the upstream driver so far as 1920x1080
> has been the maximum resolution supported by the HDMI driver and that
> resolution is supported by all VOP variants. Now with higher resolutions
> supported in the HDMI driver we have to limit the resolutions to the
> ones supported by the VOP.
>
> The actual maximum resolutions are taken from the Rockchip downstream
> Kernel.

So just so I understand it, this will allow only up to 1080p on rk3399 
or will it change something that allows higher resolutions, but with 
lower clock/pixel rates?

Thank you.

>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>
> Notes:
>      Changes since v3:
>      - new patch
>
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
>   3 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa1f4ee6d1950..96b6bd8d17803 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
>   	spin_unlock_irqrestore(&vop->irq_lock, flags);
>   }
>   
> +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> +						const struct drm_display_mode *mode)
> +{
> +	struct vop *vop = to_vop(crtc);
> +
> +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> +		return MODE_BAD_HVALUE;
> +
> +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> +		return MODE_BAD_VVALUE;
> +
> +	return MODE_OK;
> +}
> +
>   static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>   				const struct drm_display_mode *mode,
>   				struct drm_display_mode *adjusted_mode)
> @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>   }
>   
>   static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> +	.mode_valid = vop_crtc_mode_valid,
>   	.mode_fixup = vop_crtc_mode_fixup,
>   	.atomic_check = vop_crtc_atomic_check,
>   	.atomic_begin = vop_crtc_atomic_begin,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 8502849833d93..5c4875ca3f270 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -225,6 +225,8 @@ struct vop_data {
>   	const struct vop_win_data *win;
>   	unsigned int win_size;
>   	unsigned int lut_size;
> +	unsigned int max_xres;
> +	unsigned int max_yres;
>   
>   #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
>   #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 014f99e8928e3..dc1a703d9d1a8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
>   	.output = &rk3036_output,
>   	.win = rk3036_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_win_phy rk3126_win1_data = {
> @@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
>   	.output = &rk3036_output,
>   	.win = rk3126_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const int px30_vop_intrs[] = {
> @@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
>   	.output = &px30_output,
>   	.win = px30_vop_big_win_data,
>   	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_win_data px30_vop_lit_win_data[] = {
> @@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
>   	.output = &px30_output,
>   	.win = px30_vop_lit_win_data,
>   	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_scl_regs rk3066_win_scl = {
> @@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
>   	.output = &rk3066_output,
>   	.win = rk3066_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_scl_regs rk3188_win_scl = {
> @@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
>   	.win = rk3188_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
>   	.feature = VOP_FEATURE_INTERNAL_RGB,
> +	.max_xres = 2048,
> +	.max_yres = 1536,
>   };
>   
>   static const struct vop_scl_extension rk3288_win_full_scl_ext = {
> @@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
>   	.win = rk3288_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
>   	.lut_size = 1024,
> +	/*
> +	 * This is the maximum resolution for the VOPB, the VOPL can only do
> +	 * 2560x1600, but we can't distinguish them as they have the same
> +	 * compatible.
> +	 */
> +	.max_xres = 3840,
> +	.max_yres = 2160,
>   };
>   
>   static const int rk3368_vop_intrs[] = {
> @@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3368_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_intr rk3366_vop_intr = {
> @@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3368_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_output rk3399_output = {
> @@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
>   	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
>   	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>   	.lut_size = 1024,
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> @@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
>   	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
>   	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
>   	.lut_size = 256,
> +	.max_xres = 2560,
> +	.max_yres = 1600,
>   };
>   
>   static const struct vop_win_data rk3228_vop_win_data[] = {
> @@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3228_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_modeset rk3328_modeset = {
> @@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
>   	.misc = &rk3328_misc,
>   	.win = rk3328_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct of_device_id vop_driver_dt_match[] = {
-- 
Kind regards
*Dan Johansen*
Project lead of the *Manjaro ARM* project
Manjaro-ARM <https://manjaro.org>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Dan Johansen]

Den 07.02.2023 kl. 09.44 skrev Sascha Hauer:
> The different VOP variants support different maximum resolutions. Reject
> resolutions that are not supported by a specific variant.
>
> This hasn't been a problem in the upstream driver so far as 1920x1080
> has been the maximum resolution supported by the HDMI driver and that
> resolution is supported by all VOP variants. Now with higher resolutions
> supported in the HDMI driver we have to limit the resolutions to the
> ones supported by the VOP.
>
> The actual maximum resolutions are taken from the Rockchip downstream
> Kernel.

So just so I understand it, this will allow only up to 1080p on rk3399 
or will it change something that allows higher resolutions, but with 
lower clock/pixel rates?

Thank you.

>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>
> Notes:
>      Changes since v3:
>      - new patch
>
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
>   3 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa1f4ee6d1950..96b6bd8d17803 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
>   	spin_unlock_irqrestore(&vop->irq_lock, flags);
>   }
>   
> +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> +						const struct drm_display_mode *mode)
> +{
> +	struct vop *vop = to_vop(crtc);
> +
> +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> +		return MODE_BAD_HVALUE;
> +
> +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> +		return MODE_BAD_VVALUE;
> +
> +	return MODE_OK;
> +}
> +
>   static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>   				const struct drm_display_mode *mode,
>   				struct drm_display_mode *adjusted_mode)
> @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>   }
>   
>   static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> +	.mode_valid = vop_crtc_mode_valid,
>   	.mode_fixup = vop_crtc_mode_fixup,
>   	.atomic_check = vop_crtc_atomic_check,
>   	.atomic_begin = vop_crtc_atomic_begin,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 8502849833d93..5c4875ca3f270 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -225,6 +225,8 @@ struct vop_data {
>   	const struct vop_win_data *win;
>   	unsigned int win_size;
>   	unsigned int lut_size;
> +	unsigned int max_xres;
> +	unsigned int max_yres;
>   
>   #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
>   #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 014f99e8928e3..dc1a703d9d1a8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
>   	.output = &rk3036_output,
>   	.win = rk3036_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_win_phy rk3126_win1_data = {
> @@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
>   	.output = &rk3036_output,
>   	.win = rk3126_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const int px30_vop_intrs[] = {
> @@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
>   	.output = &px30_output,
>   	.win = px30_vop_big_win_data,
>   	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_win_data px30_vop_lit_win_data[] = {
> @@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
>   	.output = &px30_output,
>   	.win = px30_vop_lit_win_data,
>   	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_scl_regs rk3066_win_scl = {
> @@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
>   	.output = &rk3066_output,
>   	.win = rk3066_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>   };
>   
>   static const struct vop_scl_regs rk3188_win_scl = {
> @@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
>   	.win = rk3188_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
>   	.feature = VOP_FEATURE_INTERNAL_RGB,
> +	.max_xres = 2048,
> +	.max_yres = 1536,
>   };
>   
>   static const struct vop_scl_extension rk3288_win_full_scl_ext = {
> @@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
>   	.win = rk3288_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
>   	.lut_size = 1024,
> +	/*
> +	 * This is the maximum resolution for the VOPB, the VOPL can only do
> +	 * 2560x1600, but we can't distinguish them as they have the same
> +	 * compatible.
> +	 */
> +	.max_xres = 3840,
> +	.max_yres = 2160,
>   };
>   
>   static const int rk3368_vop_intrs[] = {
> @@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3368_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_intr rk3366_vop_intr = {
> @@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3368_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_output rk3399_output = {
> @@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
>   	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
>   	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>   	.lut_size = 1024,
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> @@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
>   	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
>   	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
>   	.lut_size = 256,
> +	.max_xres = 2560,
> +	.max_yres = 1600,
>   };
>   
>   static const struct vop_win_data rk3228_vop_win_data[] = {
> @@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
>   	.misc = &rk3368_misc,
>   	.win = rk3228_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct vop_modeset rk3328_modeset = {
> @@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
>   	.misc = &rk3328_misc,
>   	.win = rk3328_vop_win_data,
>   	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>   };
>   
>   static const struct of_device_id vop_driver_dt_match[] = {
-- 
Kind regards
*Dan Johansen*
Project lead of the *Manjaro ARM* project
Manjaro-ARM <https://manjaro.org>

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

On Tue, Feb 07, 2023 at 10:16:57AM +0100, Dan Johansen wrote:
> 
> Den 07.02.2023 kl. 09.44 skrev Sascha Hauer:
> > The different VOP variants support different maximum resolutions. Reject
> > resolutions that are not supported by a specific variant.
> > 
> > This hasn't been a problem in the upstream driver so far as 1920x1080
> > has been the maximum resolution supported by the HDMI driver and that
> > resolution is supported by all VOP variants. Now with higher resolutions
> > supported in the HDMI driver we have to limit the resolutions to the
> > ones supported by the VOP.
> > 
> > The actual maximum resolutions are taken from the Rockchip downstream
> > Kernel.
> 
> So just so I understand it, this will allow only up to 1080p on rk3399 or
> will it change something that allows higher resolutions, but with lower
> clock/pixel rates?

This patch is not about bandwidth limitations, only limitations in the
maximum resolution.

The RK3399 has two VOPs, VOPB and VOPL. The latter can only do 1080p
whereas the former can do up to 4k@30. This patch limits the allowed
resolutions to what the VOP can do. So when your application chooses
VOPB you should see 4k@30 as long as your monitor supports it.

In my testing weston has chosen VOPB and thus shows 4k@30, but I can't
tell if it does so because weston is smart enough, or just happens to
default to the VOPB.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

On Tue, Feb 07, 2023 at 10:16:57AM +0100, Dan Johansen wrote:
> 
> Den 07.02.2023 kl. 09.44 skrev Sascha Hauer:
> > The different VOP variants support different maximum resolutions. Reject
> > resolutions that are not supported by a specific variant.
> > 
> > This hasn't been a problem in the upstream driver so far as 1920x1080
> > has been the maximum resolution supported by the HDMI driver and that
> > resolution is supported by all VOP variants. Now with higher resolutions
> > supported in the HDMI driver we have to limit the resolutions to the
> > ones supported by the VOP.
> > 
> > The actual maximum resolutions are taken from the Rockchip downstream
> > Kernel.
> 
> So just so I understand it, this will allow only up to 1080p on rk3399 or
> will it change something that allows higher resolutions, but with lower
> clock/pixel rates?

This patch is not about bandwidth limitations, only limitations in the
maximum resolution.

The RK3399 has two VOPs, VOPB and VOPL. The latter can only do 1080p
whereas the former can do up to 4k@30. This patch limits the allowed
resolutions to what the VOP can do. So when your application chooses
VOPB you should see 4k@30 as long as your monitor supports it.

In my testing weston has chosen VOPB and thus shows 4k@30, but I can't
tell if it does so because weston is smart enough, or just happens to
default to the VOPB.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Jonas Karlman]

Hi Sascha,
On 2023-02-07 09:44, Sascha Hauer wrote:
> The different VOP variants support different maximum resolutions. Reject
> resolutions that are not supported by a specific variant.
> 
> This hasn't been a problem in the upstream driver so far as 1920x1080
> has been the maximum resolution supported by the HDMI driver and that
> resolution is supported by all VOP variants. Now with higher resolutions
> supported in the HDMI driver we have to limit the resolutions to the
> ones supported by the VOP.
> 
> The actual maximum resolutions are taken from the Rockchip downstream
> Kernel.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> 
> Notes:
>     Changes since v3:
>     - new patch
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa1f4ee6d1950..96b6bd8d17803 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
>  	spin_unlock_irqrestore(&vop->irq_lock, flags);
>  }
>  
> +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> +						const struct drm_display_mode *mode)
> +{
> +	struct vop *vop = to_vop(crtc);
> +
> +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> +		return MODE_BAD_HVALUE;
> +
> +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> +		return MODE_BAD_VVALUE;
> +
> +	return MODE_OK;
> +}
> +
>  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>  				const struct drm_display_mode *mode,
>  				struct drm_display_mode *adjusted_mode)
> @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> +	.mode_valid = vop_crtc_mode_valid,
>  	.mode_fixup = vop_crtc_mode_fixup,
>  	.atomic_check = vop_crtc_atomic_check,
>  	.atomic_begin = vop_crtc_atomic_begin,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 8502849833d93..5c4875ca3f270 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -225,6 +225,8 @@ struct vop_data {
>  	const struct vop_win_data *win;
>  	unsigned int win_size;
>  	unsigned int lut_size;
> +	unsigned int max_xres;
> +	unsigned int max_yres;

I would suggest using the same struct vop_rect max_input/output as the
vop2 driver instead of handling this differently between the two.

Regards,
Jonas

>  
>  #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
>  #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 014f99e8928e3..dc1a703d9d1a8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
>  	.output = &rk3036_output,
>  	.win = rk3036_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_win_phy rk3126_win1_data = {
> @@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
>  	.output = &rk3036_output,
>  	.win = rk3126_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const int px30_vop_intrs[] = {
> @@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
>  	.output = &px30_output,
>  	.win = px30_vop_big_win_data,
>  	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_win_data px30_vop_lit_win_data[] = {
> @@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
>  	.output = &px30_output,
>  	.win = px30_vop_lit_win_data,
>  	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_scl_regs rk3066_win_scl = {
> @@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
>  	.output = &rk3066_output,
>  	.win = rk3066_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_scl_regs rk3188_win_scl = {
> @@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
>  	.win = rk3188_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
>  	.feature = VOP_FEATURE_INTERNAL_RGB,
> +	.max_xres = 2048,
> +	.max_yres = 1536,
>  };
>  
>  static const struct vop_scl_extension rk3288_win_full_scl_ext = {
> @@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
>  	.win = rk3288_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
>  	.lut_size = 1024,
> +	/*
> +	 * This is the maximum resolution for the VOPB, the VOPL can only do
> +	 * 2560x1600, but we can't distinguish them as they have the same
> +	 * compatible.
> +	 */
> +	.max_xres = 3840,
> +	.max_yres = 2160,
>  };
>  
>  static const int rk3368_vop_intrs[] = {
> @@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_intr rk3366_vop_intr = {
> @@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_output rk3399_output = {
> @@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
>  	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
>  	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>  	.lut_size = 1024,
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> @@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
>  	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
>  	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
>  	.lut_size = 256,
> +	.max_xres = 2560,
> +	.max_yres = 1600,
>  };
>  
>  static const struct vop_win_data rk3228_vop_win_data[] = {
> @@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3228_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_modeset rk3328_modeset = {
> @@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
>  	.misc = &rk3328_misc,
>  	.win = rk3328_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct of_device_id vop_driver_dt_match[] = {


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Jonas Karlman]

Hi Sascha,
On 2023-02-07 09:44, Sascha Hauer wrote:
> The different VOP variants support different maximum resolutions. Reject
> resolutions that are not supported by a specific variant.
> 
> This hasn't been a problem in the upstream driver so far as 1920x1080
> has been the maximum resolution supported by the HDMI driver and that
> resolution is supported by all VOP variants. Now with higher resolutions
> supported in the HDMI driver we have to limit the resolutions to the
> ones supported by the VOP.
> 
> The actual maximum resolutions are taken from the Rockchip downstream
> Kernel.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> 
> Notes:
>     Changes since v3:
>     - new patch
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fa1f4ee6d1950..96b6bd8d17803 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
>  	spin_unlock_irqrestore(&vop->irq_lock, flags);
>  }
>  
> +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> +						const struct drm_display_mode *mode)
> +{
> +	struct vop *vop = to_vop(crtc);
> +
> +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> +		return MODE_BAD_HVALUE;
> +
> +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> +		return MODE_BAD_VVALUE;
> +
> +	return MODE_OK;
> +}
> +
>  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
>  				const struct drm_display_mode *mode,
>  				struct drm_display_mode *adjusted_mode)
> @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> +	.mode_valid = vop_crtc_mode_valid,
>  	.mode_fixup = vop_crtc_mode_fixup,
>  	.atomic_check = vop_crtc_atomic_check,
>  	.atomic_begin = vop_crtc_atomic_begin,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index 8502849833d93..5c4875ca3f270 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -225,6 +225,8 @@ struct vop_data {
>  	const struct vop_win_data *win;
>  	unsigned int win_size;
>  	unsigned int lut_size;
> +	unsigned int max_xres;
> +	unsigned int max_yres;

I would suggest using the same struct vop_rect max_input/output as the
vop2 driver instead of handling this differently between the two.

Regards,
Jonas

>  
>  #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
>  #define VOP_FEATURE_INTERNAL_RGB	BIT(1)
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 014f99e8928e3..dc1a703d9d1a8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -181,6 +181,8 @@ static const struct vop_data rk3036_vop = {
>  	.output = &rk3036_output,
>  	.win = rk3036_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3036_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_win_phy rk3126_win1_data = {
> @@ -213,6 +215,8 @@ static const struct vop_data rk3126_vop = {
>  	.output = &rk3036_output,
>  	.win = rk3126_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3126_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const int px30_vop_intrs[] = {
> @@ -340,6 +344,8 @@ static const struct vop_data px30_vop_big = {
>  	.output = &px30_output,
>  	.win = px30_vop_big_win_data,
>  	.win_size = ARRAY_SIZE(px30_vop_big_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_win_data px30_vop_lit_win_data[] = {
> @@ -356,6 +362,8 @@ static const struct vop_data px30_vop_lit = {
>  	.output = &px30_output,
>  	.win = px30_vop_lit_win_data,
>  	.win_size = ARRAY_SIZE(px30_vop_lit_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_scl_regs rk3066_win_scl = {
> @@ -479,6 +487,8 @@ static const struct vop_data rk3066_vop = {
>  	.output = &rk3066_output,
>  	.win = rk3066_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3066_vop_win_data),
> +	.max_xres = 1920,
> +	.max_yres = 1080,
>  };
>  
>  static const struct vop_scl_regs rk3188_win_scl = {
> @@ -585,6 +595,8 @@ static const struct vop_data rk3188_vop = {
>  	.win = rk3188_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
>  	.feature = VOP_FEATURE_INTERNAL_RGB,
> +	.max_xres = 2048,
> +	.max_yres = 1536,
>  };
>  
>  static const struct vop_scl_extension rk3288_win_full_scl_ext = {
> @@ -732,6 +744,13 @@ static const struct vop_data rk3288_vop = {
>  	.win = rk3288_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3288_vop_win_data),
>  	.lut_size = 1024,
> +	/*
> +	 * This is the maximum resolution for the VOPB, the VOPL can only do
> +	 * 2560x1600, but we can't distinguish them as they have the same
> +	 * compatible.
> +	 */
> +	.max_xres = 3840,
> +	.max_yres = 2160,
>  };
>  
>  static const int rk3368_vop_intrs[] = {
> @@ -833,6 +852,8 @@ static const struct vop_data rk3368_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_intr rk3366_vop_intr = {
> @@ -854,6 +875,8 @@ static const struct vop_data rk3366_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3368_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3368_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_output rk3399_output = {
> @@ -984,6 +1007,8 @@ static const struct vop_data rk3399_vop_big = {
>  	.win_size = ARRAY_SIZE(rk3399_vop_win_data),
>  	.win_yuv2yuv = rk3399_vop_big_win_yuv2yuv_data,
>  	.lut_size = 1024,
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_win_data rk3399_vop_lit_win_data[] = {
> @@ -1010,6 +1035,8 @@ static const struct vop_data rk3399_vop_lit = {
>  	.win_size = ARRAY_SIZE(rk3399_vop_lit_win_data),
>  	.win_yuv2yuv = rk3399_vop_lit_win_yuv2yuv_data,
>  	.lut_size = 256,
> +	.max_xres = 2560,
> +	.max_yres = 1600,
>  };
>  
>  static const struct vop_win_data rk3228_vop_win_data[] = {
> @@ -1029,6 +1056,8 @@ static const struct vop_data rk3228_vop = {
>  	.misc = &rk3368_misc,
>  	.win = rk3228_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3228_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct vop_modeset rk3328_modeset = {
> @@ -1100,6 +1129,8 @@ static const struct vop_data rk3328_vop = {
>  	.misc = &rk3328_misc,
>  	.win = rk3328_vop_win_data,
>  	.win_size = ARRAY_SIZE(rk3328_vop_win_data),
> +	.max_xres = 4096,
> +	.max_yres = 2160,
>  };
>  
>  static const struct of_device_id vop_driver_dt_match[] = {

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Jonas Karlman]

Hi Sascha,

On 2023-02-07 09:44, Sascha Hauer wrote:
> The Rockchip PLL drivers are currently table based and support only
> the most common pixelclocks. Discard all modes we cannot achieve
> at all. Normally the desired pixelclocks have an exact match in the
> PLL driver, nevertheless allow for a 0.1% error just in case.
> 
> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> Tested-by: Dan Johansen <strit@manjaro.org>
> Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index feba6b9becd6c..725952811752b 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
>  {
>  	struct rockchip_hdmi *hdmi = data;
>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
> -	int pclk = mode->clock * 1000;
> +	int rpclk, pclk = mode->clock * 1000;
>  	bool exact_match = hdmi->plat_data->phy_force_vendor;
>  	int i;
>  
> +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
> +	if (abs(rpclk - pclk) > pclk / 1000)
> +		return MODE_NOCLOCK;

The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.

Regards,
Jonas

> +
>  	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
>  		/*
>  		 * For vendor specific phys force an exact match of the pixelclock


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Jonas Karlman]

Hi Sascha,

On 2023-02-07 09:44, Sascha Hauer wrote:
> The Rockchip PLL drivers are currently table based and support only
> the most common pixelclocks. Discard all modes we cannot achieve
> at all. Normally the desired pixelclocks have an exact match in the
> PLL driver, nevertheless allow for a 0.1% error just in case.
> 
> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> Tested-by: Dan Johansen <strit@manjaro.org>
> Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index feba6b9becd6c..725952811752b 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
>  {
>  	struct rockchip_hdmi *hdmi = data;
>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
> -	int pclk = mode->clock * 1000;
> +	int rpclk, pclk = mode->clock * 1000;
>  	bool exact_match = hdmi->plat_data->phy_force_vendor;
>  	int i;
>  
> +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
> +	if (abs(rpclk - pclk) > pclk / 1000)
> +		return MODE_NOCLOCK;

The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.

Regards,
Jonas

> +
>  	for (i = 0; mpll_cfg[i].mpixelclock != (~0UL); i++) {
>  		/*
>  		 * For vendor specific phys force an exact match of the pixelclock

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

On Tue, Feb 07, 2023 at 10:46:49AM +0000, Jonas Karlman wrote:
> Hi Sascha,
> On 2023-02-07 09:44, Sascha Hauer wrote:
> > The different VOP variants support different maximum resolutions. Reject
> > resolutions that are not supported by a specific variant.
> > 
> > This hasn't been a problem in the upstream driver so far as 1920x1080
> > has been the maximum resolution supported by the HDMI driver and that
> > resolution is supported by all VOP variants. Now with higher resolutions
> > supported in the HDMI driver we have to limit the resolutions to the
> > ones supported by the VOP.
> > 
> > The actual maximum resolutions are taken from the Rockchip downstream
> > Kernel.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > 
> > Notes:
> >     Changes since v3:
> >     - new patch
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
> >  3 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index fa1f4ee6d1950..96b6bd8d17803 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
> >  	spin_unlock_irqrestore(&vop->irq_lock, flags);
> >  }
> >  
> > +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> > +						const struct drm_display_mode *mode)
> > +{
> > +	struct vop *vop = to_vop(crtc);
> > +
> > +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> > +		return MODE_BAD_HVALUE;
> > +
> > +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> > +		return MODE_BAD_VVALUE;
> > +
> > +	return MODE_OK;
> > +}
> > +
> >  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
> >  				const struct drm_display_mode *mode,
> >  				struct drm_display_mode *adjusted_mode)
> > @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> >  }
> >  
> >  static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> > +	.mode_valid = vop_crtc_mode_valid,
> >  	.mode_fixup = vop_crtc_mode_fixup,
> >  	.atomic_check = vop_crtc_atomic_check,
> >  	.atomic_begin = vop_crtc_atomic_begin,
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > index 8502849833d93..5c4875ca3f270 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > @@ -225,6 +225,8 @@ struct vop_data {
> >  	const struct vop_win_data *win;
> >  	unsigned int win_size;
> >  	unsigned int lut_size;
> > +	unsigned int max_xres;
> > +	unsigned int max_yres;
> 
> I would suggest using the same struct vop_rect max_input/output as the
> vop2 driver instead of handling this differently between the two.

As it was me who introduced struct vop_rect in the vop2 driver I
probably have to agree ;)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v4 1/4] drm/rockchip: vop: limit maximium resolution to hardware capabilities

[Sascha Hauer]

On Tue, Feb 07, 2023 at 10:46:49AM +0000, Jonas Karlman wrote:
> Hi Sascha,
> On 2023-02-07 09:44, Sascha Hauer wrote:
> > The different VOP variants support different maximum resolutions. Reject
> > resolutions that are not supported by a specific variant.
> > 
> > This hasn't been a problem in the upstream driver so far as 1920x1080
> > has been the maximum resolution supported by the HDMI driver and that
> > resolution is supported by all VOP variants. Now with higher resolutions
> > supported in the HDMI driver we have to limit the resolutions to the
> > ones supported by the VOP.
> > 
> > The actual maximum resolutions are taken from the Rockchip downstream
> > Kernel.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > 
> > Notes:
> >     Changes since v3:
> >     - new patch
> > 
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++++++++
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |  2 ++
> >  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 31 +++++++++++++++++++++
> >  3 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index fa1f4ee6d1950..96b6bd8d17803 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -1174,6 +1174,20 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc)
> >  	spin_unlock_irqrestore(&vop->irq_lock, flags);
> >  }
> >  
> > +static enum drm_mode_status vop_crtc_mode_valid(struct drm_crtc *crtc,
> > +						const struct drm_display_mode *mode)
> > +{
> > +	struct vop *vop = to_vop(crtc);
> > +
> > +	if (vop->data->max_xres && mode->hdisplay > vop->data->max_xres)
> > +		return MODE_BAD_HVALUE;
> > +
> > +	if (vop->data->max_yres && mode->vdisplay > vop->data->max_yres)
> > +		return MODE_BAD_VVALUE;
> > +
> > +	return MODE_OK;
> > +}
> > +
> >  static bool vop_crtc_mode_fixup(struct drm_crtc *crtc,
> >  				const struct drm_display_mode *mode,
> >  				struct drm_display_mode *adjusted_mode)
> > @@ -1585,6 +1599,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> >  }
> >  
> >  static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> > +	.mode_valid = vop_crtc_mode_valid,
> >  	.mode_fixup = vop_crtc_mode_fixup,
> >  	.atomic_check = vop_crtc_atomic_check,
> >  	.atomic_begin = vop_crtc_atomic_begin,
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > index 8502849833d93..5c4875ca3f270 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> > @@ -225,6 +225,8 @@ struct vop_data {
> >  	const struct vop_win_data *win;
> >  	unsigned int win_size;
> >  	unsigned int lut_size;
> > +	unsigned int max_xres;
> > +	unsigned int max_yres;
> 
> I would suggest using the same struct vop_rect max_input/output as the
> vop2 driver instead of handling this differently between the two.

As it was me who introduced struct vop_rect in the vop2 driver I
probably have to agree ;)

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Sascha Hauer]

On Tue, Feb 07, 2023 at 11:01:26AM +0000, Jonas Karlman wrote:
> Hi Sascha,
> 
> On 2023-02-07 09:44, Sascha Hauer wrote:
> > The Rockchip PLL drivers are currently table based and support only
> > the most common pixelclocks. Discard all modes we cannot achieve
> > at all. Normally the desired pixelclocks have an exact match in the
> > PLL driver, nevertheless allow for a 0.1% error just in case.
> > 
> > Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> > Tested-by: Dan Johansen <strit@manjaro.org>
> > Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > index feba6b9becd6c..725952811752b 100644
> > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
> >  {
> >  	struct rockchip_hdmi *hdmi = data;
> >  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
> > -	int pclk = mode->clock * 1000;
> > +	int rpclk, pclk = mode->clock * 1000;
> >  	bool exact_match = hdmi->plat_data->phy_force_vendor;
> >  	int i;
> >  
> > +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
> > +	if (abs(rpclk - pclk) > pclk / 1000)
> > +		return MODE_NOCLOCK;
> 
> The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.

That's a bit unfortunate as we can't do this check then on these SoCs.

The clock is likely actually there in the system and maybe even in the
clock driver, just not wired up to the HDMI. I don't know which one it
is though, so I am afraid there's not much I can do about it other than
just skipping the check when the clock is not there.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Sascha Hauer]

On Tue, Feb 07, 2023 at 11:01:26AM +0000, Jonas Karlman wrote:
> Hi Sascha,
> 
> On 2023-02-07 09:44, Sascha Hauer wrote:
> > The Rockchip PLL drivers are currently table based and support only
> > the most common pixelclocks. Discard all modes we cannot achieve
> > at all. Normally the desired pixelclocks have an exact match in the
> > PLL driver, nevertheless allow for a 0.1% error just in case.
> > 
> > Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> > Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
> > Tested-by: Dan Johansen <strit@manjaro.org>
> > Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > index feba6b9becd6c..725952811752b 100644
> > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
> >  {
> >  	struct rockchip_hdmi *hdmi = data;
> >  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
> > -	int pclk = mode->clock * 1000;
> > +	int rpclk, pclk = mode->clock * 1000;
> >  	bool exact_match = hdmi->plat_data->phy_force_vendor;
> >  	int i;
> >  
> > +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
> > +	if (abs(rpclk - pclk) > pclk / 1000)
> > +		return MODE_NOCLOCK;
> 
> The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.

That's a bit unfortunate as we can't do this check then on these SoCs.

The clock is likely actually there in the system and maybe even in the
clock driver, just not wired up to the HDMI. I don't know which one it
is though, so I am afraid there's not much I can do about it other than
just skipping the check when the clock is not there.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Jonas Karlman]

Hi Sascha,

On 2023-02-07 13:51, Sascha Hauer wrote:
> On Tue, Feb 07, 2023 at 11:01:26AM +0000, Jonas Karlman wrote:
>> Hi Sascha,
>>
>> On 2023-02-07 09:44, Sascha Hauer wrote:
>>> The Rockchip PLL drivers are currently table based and support only
>>> the most common pixelclocks. Discard all modes we cannot achieve
>>> at all. Normally the desired pixelclocks have an exact match in the
>>> PLL driver, nevertheless allow for a 0.1% error just in case.
>>>
>>> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>>> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
>>> Tested-by: Dan Johansen <strit@manjaro.org>
>>> Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> index feba6b9becd6c..725952811752b 100644
>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
>>>  {
>>>  	struct rockchip_hdmi *hdmi = data;
>>>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
>>> -	int pclk = mode->clock * 1000;
>>> +	int rpclk, pclk = mode->clock * 1000;
>>>  	bool exact_match = hdmi->plat_data->phy_force_vendor;
>>>  	int i;
>>>  
>>> +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
>>> +	if (abs(rpclk - pclk) > pclk / 1000)
>>> +		return MODE_NOCLOCK;
>>
>> The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.
> 
> That's a bit unfortunate as we can't do this check then on these SoCs.
> 
> The clock is likely actually there in the system and maybe even in the
> clock driver, just not wired up to the HDMI. I don't know which one it
> is though, so I am afraid there's not much I can do about it other than
> just skipping the check when the clock is not there.

For rk3228/rk3328 I think just skipping the rate check when there is
no ref_clk should be fine.

The clock is provided by phy-rockchip-inno-hdmi.c for rk3228/rk3328. And
I recall we used to add ref/vpll clock to device tree to do something
similar in LibreELEC with our initial work-in-progress HDMI2.0 patches.

We have since then opted to just filter modes based on clk_round_rate
for !phy_force_vendor and instead extend the inno-hdmi pll table with
common hdmi/dvi clock rates, see [1] for the current state of RK HDMI2.0
related patches we use in LibreELEC.

Hopefully I can find some time in coming weeks to restart the work of
testing and sending those patches upstream, rebased on top of this series.

[1] https://github.com/Kwiboo/linux-rockchip/compare/v6.2-rc7...rockchip-6.2-hdmi2.0

Regards,
Jonas

> 
> Sascha
> 

Re: [PATCH v4 4/4] drm/rockchip: dw_hdmi: discard modes with unachievable pixelclocks

[Jonas Karlman]

Hi Sascha,

On 2023-02-07 13:51, Sascha Hauer wrote:
> On Tue, Feb 07, 2023 at 11:01:26AM +0000, Jonas Karlman wrote:
>> Hi Sascha,
>>
>> On 2023-02-07 09:44, Sascha Hauer wrote:
>>> The Rockchip PLL drivers are currently table based and support only
>>> the most common pixelclocks. Discard all modes we cannot achieve
>>> at all. Normally the desired pixelclocks have an exact match in the
>>> PLL driver, nevertheless allow for a 0.1% error just in case.
>>>
>>> Tested-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
>>> Tested-by: Michael Riesch <michael.riesch@wolfvision.net>
>>> Tested-by: Dan Johansen <strit@manjaro.org>
>>> Link: https://lore.kernel.org/r/20230118132213.2911418-4-s.hauer@pengutronix.de>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> index feba6b9becd6c..725952811752b 100644
>>> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
>>> @@ -256,10 +256,14 @@ dw_hdmi_rockchip_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
>>>  {
>>>  	struct rockchip_hdmi *hdmi = data;
>>>  	const struct dw_hdmi_mpll_config *mpll_cfg = rockchip_mpll_cfg;
>>> -	int pclk = mode->clock * 1000;
>>> +	int rpclk, pclk = mode->clock * 1000;
>>>  	bool exact_match = hdmi->plat_data->phy_force_vendor;
>>>  	int i;
>>>  
>>> +	rpclk = clk_round_rate(hdmi->ref_clk, pclk);
>>> +	if (abs(rpclk - pclk) > pclk / 1000)
>>> +		return MODE_NOCLOCK;
>>
>> The ref_clk is optional and rk3228/rk3328 dts do not supply a ref or vpll clock.
> 
> That's a bit unfortunate as we can't do this check then on these SoCs.
> 
> The clock is likely actually there in the system and maybe even in the
> clock driver, just not wired up to the HDMI. I don't know which one it
> is though, so I am afraid there's not much I can do about it other than
> just skipping the check when the clock is not there.

For rk3228/rk3328 I think just skipping the rate check when there is
no ref_clk should be fine.

The clock is provided by phy-rockchip-inno-hdmi.c for rk3228/rk3328. And
I recall we used to add ref/vpll clock to device tree to do something
similar in LibreELEC with our initial work-in-progress HDMI2.0 patches.

We have since then opted to just filter modes based on clk_round_rate
for !phy_force_vendor and instead extend the inno-hdmi pll table with
common hdmi/dvi clock rates, see [1] for the current state of RK HDMI2.0
related patches we use in LibreELEC.

Hopefully I can find some time in coming weeks to restart the work of
testing and sending those patches upstream, rebased on top of this series.

[1] https://github.com/Kwiboo/linux-rockchip/compare/v6.2-rc7...rockchip-6.2-hdmi2.0

Regards,
Jonas

> 
> Sascha
> 


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip