Date:   Mon, 17 Jul 2017 21:22:10 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     lkml <linux-kernel@...r.kernel.org>
Cc:     John Stultz <john.stultz@...aro.org>,
        Daniel Vetter <daniel.vetter@...el.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Sean Paul <seanpaul@...omium.org>,
        David Airlie <airlied@...ux.ie>,
        Rob Clark <robdclark@...il.com>,
        Xinliang Liu <xinliang.liu@...aro.org>,
        Xinliang Liu <z.liuxinliang@...ilicon.com>,
        Rongrong Zou <zourongrong@...il.com>,
        Xinwei Kong <kong.kongxinwei@...ilicon.com>,
        Chen Feng <puck.chen@...ilicon.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>,
        Archit Taneja <architt@...eaurora.org>,
        dri-devel@...ts.freedesktop.org
Subject: [RFC][PATCH v2] drm: kirin: Add mode_valid logic to avoid mode clocks we can't generate

Currently the hikey dsi logic cannot generate accurate byte
clocks values for all pixel clock values. Thus if a mode clock
is selected that cannot match the calculated byte clock, the
device will boot with a blank screen.

This patch uses the new mode_valid callback (many thanks to
Jose Abreu for upstreaming it!) to ensure we don't select
modes we cannot generate.

NOTE: Stylistically I suspect there are better ways to do what
I'm trying to do here. The encoder -> crtc bit is terrible, and
getting the crtc adjusted mode from the encoder logic feels
less then ideal. So feedback would be greatly appreciated!

Cc: Daniel Vetter <daniel.vetter@...el.com>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Sean Paul <seanpaul@...omium.org>
Cc: David Airlie <airlied@...ux.ie>
Cc: Rob Clark <robdclark@...il.com>
Cc: Xinliang Liu <xinliang.liu@...aro.org>
Cc: Xinliang Liu <z.liuxinliang@...ilicon.com>
Cc: Rongrong Zou <zourongrong@...il.com>
Cc: Xinwei Kong <kong.kongxinwei@...ilicon.com>
Cc: Chen Feng <puck.chen@...ilicon.com>
Cc: Jose Abreu <Jose.Abreu@...opsys.com>
Cc: Archit Taneja <architt@...eaurora.org>
Cc: dri-devel@...ts.freedesktop.org
Signed-off-by: John Stultz <john.stultz@...aro.org>
---
v2: Reworked to calculate if modeclock matches the phy's byteclock,
    rather then using a whitelist of known modes.
---
 drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c    | 45 +++++++++++++++++++++++++
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  8 +++++
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |  2 ++
 3 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
index f77dcfa..9a553e7 100644
--- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
+++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
@@ -24,6 +24,7 @@
 #include <drm/drm_encoder_slave.h>
 #include <drm/drm_atomic_helper.h>
 
+#include "kirin_drm_drv.h"
 #include "dw_dsi_reg.h"
 
 #define MAX_TX_ESC_CLK		10
@@ -603,6 +604,49 @@ static void dsi_encoder_enable(struct drm_encoder *encoder)
 	dsi->enable = true;
 }
 
+static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder *encoder,
+					const struct drm_display_mode *mode)
+{
+	struct dw_dsi *dsi = encoder_to_dsi(encoder);
+	struct drm_crtc *crtc = NULL;
+	struct mipi_phy_params phy;
+	u32 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+	u32 adjusted_clock;
+	u32 req_kHz, act_kHz, lane_byte_clk_kHz;
+
+	/* Figure out what the adjusted modeclock will be */
+	/* XXX There's got to be a better way to go encoder->crtc */
+	drm_for_each_crtc(crtc, encoder->dev)
+		if (crtc)
+			break;
+	if (crtc)
+		adjusted_clock = kirin_ade_adj_mode_clk(crtc, mode->clock);
+	else
+		adjusted_clock = mode->clock;
+
+	/* Calculate the lane byte clk using the adjusted mode clk */
+	memset(&phy, 0, sizeof(phy));
+	req_kHz = adjusted_clock * bpp / dsi->lanes;
+	act_kHz = dsi_calc_phy_rate(req_kHz, &phy);
+	lane_byte_clk_kHz = act_kHz / 8;
+
+	DRM_DEBUG_DRIVER("Checking mode %ix%i-%i@%i clock: %i adj_clock: %i...",
+			mode->hdisplay, mode->vdisplay, bpp,
+			drm_mode_vrefresh(mode), mode->clock, adjusted_clock);
+
+	/*
+	 * Make sure the adjused mode clock and the lane byte clk
+	 * have a common denominator base frequency
+	 */
+	if (adjusted_clock/dsi->lanes == lane_byte_clk_kHz/3) {
+		DRM_DEBUG_DRIVER("OK!\n");
+		return MODE_OK;
+	}
+
+	DRM_DEBUG_DRIVER("BAD!\n");
+	return MODE_BAD;
+}
+
 static void dsi_encoder_mode_set(struct drm_encoder *encoder,
 				 struct drm_display_mode *mode,
 				 struct drm_display_mode *adj_mode)
@@ -622,6 +666,7 @@ static int dsi_encoder_atomic_check(struct drm_encoder *encoder,
 
 static const struct drm_encoder_helper_funcs dw_encoder_helper_funcs = {
 	.atomic_check	= dsi_encoder_atomic_check,
+	.mode_valid	= dsi_encoder_mode_valid,
 	.mode_set	= dsi_encoder_mode_set,
 	.enable		= dsi_encoder_enable,
 	.disable	= dsi_encoder_disable
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
index 074b0af..dffcf76 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
@@ -178,6 +178,14 @@ static void ade_init(struct ade_hw_ctx *ctx)
 			FRM_END_START_MASK, REG_EFFECTIVE_IN_ADEEN_FRMEND);
 }
 
+u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk)
+{
+	struct ade_crtc *acrtc = to_ade_crtc(crtc);
+	struct ade_hw_ctx *ctx = acrtc->ctx;
+
+	return clk_round_rate(ctx->ade_pix_clk, modeclk * 1000) / 1000;
+}
+
 static void ade_set_pix_clk(struct ade_hw_ctx *ctx,
 			    struct drm_display_mode *mode,
 			    struct drm_display_mode *adj_mode)
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
index 7f60c649..85f69ee 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
@@ -27,4 +27,6 @@ struct kirin_drm_private {
 
 extern const struct kirin_dc_ops ade_dc_ops;
 
+u32 kirin_ade_adj_mode_clk(struct drm_crtc *crtc, u32 modeclk);
+
 #endif /* __KIRIN_DRM_DRV_H__ */
-- 
2.7.4

Powered by blists - more mailing lists